Shortcuts redefinition after MapFrame reinitialization

[akks]

How to reproduce:

• Open some area in josm
• Delete layer
• Open it again
• Look at console :)

Problem exist for multikey core shortcuts and some plugins. Does not now - should Shiortcut.java or all calling modules be patched.

Keystroke shift pressed S is already assigned to MultikeyActionorg.openstreetmap.josm.gui.dialogs.LayerListDialog$ShowHideLayerAction@1db4b415, will be overridden by MultikeyActionorg.openstreetmap.josm.gui.dialogs.LayerListDialog$ShowHideLayerAction@1db4b415
Keystroke shift pressed A is already assigned to MultikeyActionorg.openstreetmap.josm.gui.dialogs.LayerListDialog$ActivateLayerAction@45609812, will be overridden by MultikeyActionorg.openstreetmap.josm.gui.dialogs.LayerListDialog$ActivateLayerAction@489fe953
Keystroke ctrl alt pressed J is already assigned to MultikeyActionorg.openstreetmap.josm.gui.layer.JumpToMarkerActions$JumpToNextMarker@5e56ff18, will be overridden by MultikeyActionorg.openstreetmap.josm.gui.layer.JumpToMarkerActions$JumpToNextMarker@2185b10b
Keystroke ctrl alt pressed E is already assigned to MultikeyActionorg.openstreetmap.josm.gui.dialogs.FilterDialog$EnableFilterAction@57d62a85, will be overridden by MultikeyActionorg.openstreetmap.josm.gui.dialogs.FilterDialog$EnableFilterAction@2d81aed0
Keystroke ctrl alt pressed H is already assigned to MultikeyActionorg.openstreetmap.josm.gui.dialogs.FilterDialog$HidingFilterAction@608d116e, will be overridden by MultikeyActionorg.openstreetmap.josm.gui.dialogs.FilterDialog$HidingFilterAction@4d737e47
Keystroke ctrl alt pressed R is already assigned to relcontext.RelContextDialog$EnterRoleAction@2cc47220, will be overridden by relcontext.RelContextDialog$EnterRoleAction@4e69048b
Registered toolbar action without name: relcontext.RelContextDialog$EnterRoleAction
Keystroke pressed EQUALS is already assigned to relcontext.actions.AddRemoveMemberAction@12bc8f01, will be overridden by relcontext.actions.AddRemoveMemberAction@50a969e4
Registered toolbar action relcontext/addremove overwritten: relcontext.actions.AddRemoveMemberAction gets relcontext.actions.AddRemoveMemberAction
Keystroke ctrl alt pressed N is already assigned to relcontext.actions.CreateRelationAction@2b2143d8, will be overridden by relcontext.actions.CreateRelationAction@39673d71


Registered toolbar action data/relation overwritten: relcontext.actions.CreateRelationAction gets relcontext.actions.CreateRelationAction
Keystroke ctrl pressed B is already assigned to relcontext.actions.CreateMultipolygonAction@1e235551, will be overridden by relcontext.actions.CreateMultipolygonAction@621f6c18
Registered toolbar action data/multipolygon overwritten: relcontext.actions.CreateMultipolygonAction gets relcontext.actions.CreateMultipolygonAction
Keystroke ctrl alt pressed F is already assigned to relcontext.actions.FindRelationAction@1922e15b, will be overridden by relcontext.actions.FindRelationAction@588f1ed7
Registered toolbar action relcontext/find overwritten: relcontext.actions.FindRelationAction gets relcontext.actions.FindRelationAction

[stoecker]

Calling modules must be patched to either unregister their keys when MapFrame is closed or only initialize once. For Multikeys I would say unregistering would be proper fix.

These do not affect josm behaviour a lot, but should be fixed nevertheless.

[akks]

All of these actions are created with ToggleDialog instances (in constructor), so it is hard to find suitable time for unregistering.
Do you propose to register then on showing dialog and unregister on hiding? Filters or Layers, for example?

[stoecker]

The errors are produced, when the same class overwrites itself on next construction. So either it must be unregistered when closing dialogs or it must be registered only once. But multiple registering and leaving the shortcut assigned is wrong.

If unregistering or static single-assigned class is correct depends on the fact if shortcut has a meaning when dialog is closed.

[akks]

Yes. But I do not know exactly does if it have such meaning. It need some experiments for each case or someone who code or use them intensively.

[akks]

As I think, switching layers and filters should work even with hidden toggledialogs *(they do work now).

But the shortcuts need to be relinked to new action when dialog is reinitialized... So constructor should unregister them first ant then register new. Maybe it is simpler to support such functionality as Main.reRegisterActionShortcut?

[akks]

I looked at the code of Main.registerShortcut and realized that unregistering shortcut before registering it again is meaningless. (inputMap.remove + inputMap.put = only inputMap.put), so only thing we need is to supress the warning for certain calls by adding some boolean parameter to registerActionShortcut.
Do we need to remove actions from actionMap or they are safely replaced too?

Singleton does not work for considered case, since actions really need to be recreated (to match GUI tables and so on).
Removing shortcuts when hiding dialogs is not good because shortcuts like Shift-A are better to work in hidden state (they do work now).

[stoecker]

Replying to akks:

Singleton does not work for considered case, since actions really need to be recreated (to match GUI tables and so on).
Removing shortcuts when hiding dialogs is not good because shortcuts like Shift-A are better to work in hidden state (they do work now).

If MapFrame is destroyed when last layer is closed, e.g. the layer shortcuts can't really work anymore, so the should be unregistered at the proper place.

So this does not depend on visibility of toggle dialog, but on visibility of map.

[akks]

Replying to stoecker:

Replying to akks:

Singleton does not work for considered case, since actions really need to be recreated (to match GUI tables and so on).
Removing shortcuts when hiding dialogs is not good because shortcuts like Shift-A are better to work in hidden state (they do work now).

If MapFrame is destroyed when last layer is closed, e.g. the layer shortcuts can't really work anymore, so the should be unregistered at the proper place.

So this does not depend on visibility of toggle dialog, but on visibility of map.

Can I handle changing mapframe or dialog destruction in ToggleDialogs?
Oh, it seems I have found ready solution - override destroy(), like in ​LayerListDialog. Am I right? If needed, I can implement it (please let me know if someone started it).

[akks]

In 5018/josm:

fix "Shortcut redefinition" warnings after MapFrame reinitialization - see #7424 (core part)

[akks]

Core warnings are eliminated. Please have a look at the changes when you have time... I hope it will not break anything.

[akks]

Reltoolbox fixed in [o27927].

Only routing remains, but I do not know exactly how to fix it:

Keystroke shift ctrl alt pressed R is already assigned to org.openstreetmap.josm.gui.dialogs.ToggleDialog$ToggleDialogAction@12302ae, will be overridden by org.openstreetmap.josm.gui.dialogs.ToggleDialog$ToggleDialogAction@a14e50

[akks]

Sorry, it seems that routing plugin was tested with old JAR. I published newly compiled version and there are
no more shortcut warnings now. The ticket can be closed after someone looks at the core changes [5018] :)