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

[Jose Pereda]

On Tue, 18 Jan 2022 17:26:15 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 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