On Sat, 10 Dec 2022 10:38:07 GMT, Dean Wookey <dwoo...@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

Reply via email to