RFR: 8296409: Multiple copies of accelerator change listeners are added to MenuItems, but only 1 is removed [v3]

Andy Goryachev angorya at openjdk.org
Mon Dec 12 17:47:32 UTC 2022


On Sat, 10 Dec 2022 10:38:07 GMT, Dean Wookey <dwookey at openjdk.org> 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.
>
> Dean Wookey has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains four commits:
> 
>  - Implemented alternative fix
>  - Merge branch 'master' of https://github.com/openjdk/jfx into JDK-8296409
>    
>    # Conflicts:
>    #	modules/javafx.controls/src/main/java/javafx/scene/control/skin/MenuButtonSkinBase.java
>  - Added changing scene tests for accelerator change listeners
>  - 8296409: Stop additional change listeners being added

looks much leaner now, thanks!

-------------

Marked as reviewed by angorya (Committer).

PR: https://git.openjdk.org/jfx/pull/937


More information about the openjfx-dev mailing list