On Fri, 4 Nov 2022 16:07:54 GMT, Dean Wookey <[email protected]> wrote:
> When menu buttons are added and removed from the scene, an accelerator change
> listener is added to each menu item in the menu. There is nothing stopping
> the same change listener being added multiple times.
>
> MenuButtonSkinBase calls the
> ControlAcceleratorSupport.addAcceleratorsIntoScene(getSkinnable().getItems(),
> getSkinnable()); method each time the button is added to the scene, but that
> method itself also registers a listener to call itself. Each time the button
> is added to the scene, the method is called at least twice.
>
> When it's removed from the scene, the remove accelerator method is also
> called twice, but only the first call removes a change listener attached to
> the accelerator because the first call removes the entry from the hashmap
> changeListenerMap. The second call finds nothing in the map, and doesn't
> remove the additional instance.
>
> This pull request just removes the redundant code in the MenuButtonSkinBase.
modules/javafx.controls/src/main/java/javafx/scene/control/skin/MenuButtonSkinBase.java
line 154:
> 152: sceneChangeListener = (scene, oldValue, newValue) -> {
> 153: if (oldValue != null) {
> 154:
> ControlAcceleratorSupport.removeAcceleratorsFromScene(getSkinnable().getItems(),
> oldValue);
will it handle a case where the menu button gets attached to a different scene?
could you add a second test for this scenario please?
And i wonder if the problem is in ControlAcceleratorSupport rather than here.
We do have a similar code in Control:380, do we have a problem there?
-------------
PR: https://git.openjdk.org/jfx/pull/937
