On Tue, 18 Jan 2022 17:26:15 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 refreshed the contents of this pull request, and previous
> commits have been removed. The incremental views will show differences
> compared to the previous content of the PR. The pull request contains one new
> commit since the last revision:
>
> Re-instating fix for Cmd shortcut being handled by two windows
Fix looks good. Test fails without it, works with it. I've added just a few
minor comments about formatting.
tests/system/src/test/java/test/robot/javafx/scene/DoubleShortcutProcessing.java
line 53:
> 51: // When a key equivalent closes a window it can be passed
> 52: // to the new key window and processed twice.
> 53: public class DoubleShortcutProcessing {
Rename to `DoubleShortcutProcessingTest`?
tests/system/src/test/java/test/robot/javafx/scene/DoubleShortcutProcessing.java
line 53:
> 51: // When a key equivalent closes a window it can be passed
> 52: // to the new key window and processed twice.
> 53: public class DoubleShortcutProcessing {
When I launch the test with:
sh gradlew -PUSE_ROBOT=true -PFULL_TEST=true :systemTests:test
--tests=test.robot.javafx.scene.DoubleShortcutProcessing
I get the system dialog "Accessibility Access (Events)" with the message
"'java' would like to control this computer using accessibility features."
After adding java to Security&Privacy->accessibility, the test works fine.
Maybe this need to be added to the javadoc of the test.
tests/system/src/test/java/test/robot/javafx/scene/DoubleShortcutProcessing.java
line 63:
> 61: @Test
> 62: void testDoubleShortcut()
> 63: {
Minor: move the opening curly brace to the line above
tests/system/src/test/java/test/robot/javafx/scene/DoubleShortcutProcessing.java
line 79:
> 77: @AfterAll
> 78: static void exit() {
> 79: Platform.runLater(() -> {
It can be simplified with a method reference
tests/system/src/test/java/test/robot/javafx/scene/DoubleShortcutProcessing.java
line 116:
> 114:
> 115: public void startTest()
> 116: {
Move curly brace to line above
tests/system/src/test/java/test/robot/javafx/scene/DoubleShortcutProcessing.java
line 133:
> 131:
> 132: public boolean failed()
> 133: {
same here
-------------
PR: https://git.openjdk.java.net/jfx/pull/715
