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
The fix looks good to me. It looks like we have a build dependency problem with
the new JUnit5 Assumptions API. I get the following compilation error on my
system when trying to run the system tests:
gradle --info -PFULL_TEST=true -PUSE_ROBOT=true :systemTests:test --tests
DoubleShortcutProcessing
...
> Task :systemTests:compileTestJava
tests/system/src/test/java/test/robot/javafx/scene/DoubleShortcutProcessing.java:64:
error: cannot access TestAbortedException
Assumptions.assumeTrue(PlatformUtil.isMac());
^
class file for org.opentest4j.TestAbortedException not found
The following fixes it, although the fact that this is needed at test
compilation time seems like an unfortunate choice on the part of JUnit.
--- a/build.gradle
+++ b/build.gradle
@@ -1969,7 +1969,7 @@ allprojects {
testRuntimeOnly group: "org.junit.platform", name:
"junit-platform-commons", version: "1.8.1"
testRuntimeOnly group: "org.junit.platform", name:
"junit-platform-engine", version: "1.8.1"
testRuntimeOnly group: "org.junit.vintage", name:
"junit-vintage-engine", version: "5.8.1"
- testRuntimeOnly group: "org.opentest4j", name: "opentest4j", version:
"1.2.0"
+ testImplementation group: "org.opentest4j", name: "opentest4j",
version: "1.2.0"
It seems odd that this doesn't fail in the GitHub action run, since it fails
for me even running `gradle test` (the systemTests project is compiled, but not
executed, even when `FULL_TEST` is not set to true).
I'd prefer this dependency issue to be fixed by a separate bug, which I will
file today.
-------------
PR: https://git.openjdk.java.net/jfx/pull/715
