On Wed, 19 Jan 2022 16:25:17 GMT, Martin Fox <d...@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
