On Tue, 8 Nov 2022 10:07:54 GMT, Dean Wookey <dwoo...@openjdk.org> wrote:
>> 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. > > Here is an alternative fix: > https://github.com/openjdk/jfx/commit/80971d89c46f3f74cb8584d4907cc6818809e2f6 > I just reverted the MenuButtonSkinBase changes. The tests pass with this fix, > or with both. > > I'm a little more nervous about that one because I had to remove the code > that removed and then added a scene listener. I'm not sure why it was done > because the listener is independent of a specific scene. > > Basically, that fix only does the install if the install wasn't already done > on that anchor meaning it can get called any number of times safely stopping > duplicate accelerators being added. > > I would go with either both fixes together, or just the MenuButtonSkinBase > one. please forgive me - it might take me a bit longer to review. ------------- PR: https://git.openjdk.org/jfx/pull/937
