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

[Dean Wookey]

On Fri, 4 Nov 2022 21:14:20 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> Dean Wookey has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Added changing scene tests for accelerator change listeners
>
> 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?

The code in ControlAcceleratorSupport adds its own scene listeners for each node added. MenuButtonSkinBase does the same, unlike Control which only calls it once.

I think we could fix it in ControlAcceleratorSupport alternatively or in addition to this fix. The code in ControlAcceleratorSupport appears to do everything this would've done.

I've added the test for changing scenes.

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

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