[jfx18] RFR: 8205915: [macOS] Accelerator assigned to button in dialog fires menuItem in owning stage [v4]

[Kevin Rushforth]

On Wed, 19 Jan 2022 16:25:17 GMT, Martin Fox <duke at openjdk.java.net> wrote:

>> When a window is closed while handling performKeyEquivalent the same NSEvent might be passed to the new key window causing it to being processed twice. Long ago a fix was put in for this case; when the GlassWindow was closed a flag was set to ensure that we would return YES from performKeyEquivalent. To fix RT-39813 the closing of the window was deferred causing the flag to be set too late. This PR simply sets that flag when we schedule the close event instead of when the OS actually closes the window.
>> 
>> This is a spot-fix for a larger problem, namely that we have no way of knowing whether a performKeyEquivalent event was consumed or not. The changes for fixing that are in PR #694. The changes got bundled into that PR only because there's a lot of files involved and the exact same code paths are touched.
>> 
>> System test is included (I'm surprised, it really is possible to generate Cmd+Enter using a Robot). This is new territory for me so I have a manual test I can submit as a backup.
>
> Martin Fox has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Renamed test to match existing conventions along with minor cleanup.

The fix and test both look good, with one requested code style change. I can confirm that the test fails without the fix and passes with the fix.

Also, In order to avoid problems with our CI and nightly build, I want #720 to be integrated before this PR.

tests/system/src/test/java/test/robot/javafx/scene/DoubleShortcutProcessingTest.java line 25:

> 23:  * questions.
> 24:  */
> 25: package test.robot.javafx.scene;

Very minor: we usually add a blank line between the copyright header block and the `package` statement. I only mention it because I have another change to request.

tests/system/src/test/java/test/robot/javafx/scene/DoubleShortcutProcessingTest.java line 67:

> 65:         waitForLatch(dialogLatch, 5, "Dialog never received shortcut");
> 66:         if (testApp.failed())
> 67:             Assertions.fail("performKeyEquivalent was handled twice in separate windows");

Please add curly braces around the target of the `if` statement. The only exception to this rule for Java code is where the statement is short enough to be on the same line as the `if`.

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

PR: https://git.openjdk.java.net/jfx/pull/715