[patch] Not all JosmActions registering in toolbar

[gaben]

Changes made in r18321 blocks some core and custom actions, possibly added by a plugin (MainApplication.getToolbar().addCustomButton()) from registering into the toolbar settings.

See the screenshot

[GerdP]

Cannot reproduce this with r18303, tried my normal config and also a clean one.

[gaben]

Until r18320 is fine, the problem affects r18321 and later revisions.

[GerdP]

Ah, yes, sorry, got the version numbers wrong.

[GerdP]

Hmm, I don't understand why e.g. "Orthogonalize Shape / Undo" no longer appears and I think it was never meant to appear at that place. Instead it should appear somewehere under Tools.

[gaben]

I have no idea either. Also don't know why the phone action (provided by my plugin) forego the "move up" action. I think the move actions supposed to follow each other as they created at the same time(?).

The whole thing affects me in a way, that I can't use the JOSM core toolbar settings. So either I need to code it myself or have to choose fixing the button into the toolbar. Mostly inconvenience, but bothering.

[skyper]

See also #6725 and #14541.

[GerdP]

@gaben: Problem was this part of the code:

        for (Map.Entry<String, Action> a : regactions.entrySet()) {
            if (actions.get(a.getKey()) == null) {
                rootActionsNode.add(new DefaultMutableTreeNode(a.getValue()));
            }
        }

With my changes the actions.get(a.getKey()) == null was never true and thus only actions which appear in a menu were added. I've renamed actions to actionsInMenu to make that clearer and now the map is cleared first in loadActions().

[GerdP]

Have to think again about this. The new patch probably re-introduces the memory leak...

[GerdP]

2nd patch should also fix the memory leak as the map itself should be GCed now.
There's probably no need to use a ConcurrentHashMap, but my understanding of that is small.

[gaben]

[patch v2]

• Something strange going on. Now I got the cycle layer actions in the toolbar settings. That's fine, but where were they until now? They are missing completely in previous versions too. They seems to appear based on layer availability.
• The actions class variable was moved, but javadoc remains and now it is javadoc of regactions variable. Is this intended?

• unrelated, but the public getToolString() javadoc is missing

I can't comment on other changes, as I don't understand much of the GUI code :(

[GerdP]

The javadoc is OK for regactions, too.
Does the patch solve your plugin problem?

[gaben]

Yes, thank you for the quick fix!

[GerdP]

OK, I think I'll wait a few days before committing this. Or do you think it is worth a hotfix?

[gaben]

Ah, you are kind. I'm fine with waiting, as the plugin is not released yet.

It will be my first public code repository, and I'm not sure about licensing stuff and also accidentally used my private email in commits, so I need to rewrite the whole repo history before making it public... Otherwise the plugin is usable. ​See #15250 for request.

[GerdP]

In 18362/josm:

fix #21730: Not all JosmActions registering in toolbar

• separate again between all registered actions and those which appear in menus (regression from r18321)
• replace global map actions by local one to avoid memory leak