RFR: JDK-8302618: [macOS] Problem typing uppercase letters with java.awt.Robot when moving mouse [v7]
Alexey Ivanov
aivanov at openjdk.org
Mon Jul 24 18:33:50 UTC 2023
On Mon, 17 Jul 2023 19:09:29 GMT, Harshitha Onkar <honkar at openjdk.org> wrote:
>> **Problem:**
>>
>> On macOS, Robot erroneously produces lowercase letter when mouse is moved (manually) in unison with Robot's keyEvents. This issue was originally logged by a developer of an on-screen accessibility keyboard - TouchBoard. Originally reported at https://github.com/adoptium/adoptium-support/issues/710
>>
>> This issue is reproducible on JDK versions 22 to 11, but works fine on JDK-8. (details below)
>>
>> This issue is not restricted to the Shift modifier key and causes problems with other modifier keys as well and in some scenarios without any external mouse movement.
>>
>> - This works correctly on JDK-8 up to JDK-9+129 when Accessibility APIs (AXUIElementCreateSystemWide/ AXUIElementPostKeyboardEvent) were used. Later on it was changed to CGEvents.
>>
>> - With the present code, the issue occurs at [CRobot.m#L295](https://github.com/openjdk/jdk/blob/ac6af6a64099c182e982a0a718bc1b780cef616e/src/java.desktop/macosx/native/libawt_lwawt/awt/CRobot.m#L295.) The flags gets reset or cleared when mouse is moved physically in unison with Robot's key events.
>>
>> - The physical mouse movement causes the event flags to be reset.
>>
>> **Impact:**
>>
>> Modifier keys don't work as expected when using Robot with any simultaneous physical mouse movement and in case of TouchBoard, this behavior breaks the usability of the on-screen a11y keyboard. There is no known workaround for this particular use case except for reverting to JDK-8. More details on this use case [here.](https://github.com/adoptium/adoptium-support/issues/710#issuecomment-1594103280)
>>
>> **Proposed Fix:**
>>
>> - In order to avoid resetting of the CGEventFlags here [CRobot.m#L295](https://github.com/openjdk/jdk/blob/ac6af6a64099c182e982a0a718bc1b780cef616e/src/java.desktop/macosx/native/libawt_lwawt/awt/CRobot.m#L295.), the CGEvent flag state is obtained in `initRobot` (stored in initFlags) which is later used within `CRobot_keyEvent`.
>>
>> - The incoming keyCode is used to determine whether it is a modifier key and the corresponding modifierFlagMask is either added or cleared from the initFlags based on whether the modifier key was pressed or released.
>>
>> - Finally, only the required and known flag bits from initFlag are copied over to local flag which is used in `CGEventSetFlags()`.
>>
>> **Testing:**
>>
>> The newly added test - RobotModifierMaskTest.java tests for Shift, Caps, Control, Option and Command keys.
>> The test runs in 2 modes - as automated and manual test.
>>
>> CASE 1 : ...
>
> Harshitha Onkar has updated the pull request incrementally with one additional commit since the last revision:
>
> test changes: added CountDownLatch
src/java.desktop/macosx/native/libawt_lwawt/awt/CRobot.m line 317:
> 315: initFlags = keyPressed
> 316: ? (initFlags | flagMaskValue) // add flag bits if modifier key pressed
> 317: : (initFlags & ~flagMaskValue); // clear flag bits if modifier key released
`initFlags` is modified as each event is generated; does it store the *init* flags?
src/java.desktop/macosx/native/libawt_lwawt/awt/CRobot.m line 322:
> 320:
> 321: CGEventFlags flags = CGEventSourceFlagsState(kCGEventSourceStateHIDSystemState);
> 322: flags = (initFlags & allModifiersMask) | (flags & (~allModifiersMask));
Suggestion:
flags = (initFlags & allModifiersMask) | (flags & (~allModifiersMask));
test/jdk/java/awt/Robot/RobotModifierMaskTest.java line 101:
> 99: countDownLatch = new CountDownLatch(1);
> 100: invokeAndWait(RobotModifierMaskTest::createInstructionsUI);
> 101: robot.waitForIdle();
Technically, `waitForIdle` isn't needed here: you start waiting for the user to click the Start button. Without it, the waiting time may be reduced by a small amount of time.
test/jdk/java/awt/Robot/RobotModifierMaskTest.java line 107:
> 105: if (!testStarted) {
> 106: throw new RuntimeException("Test Failed: Manual test timed out!!");
> 107: }
Suggestion:
if (!countDownLatch.await(2, TimeUnit.MINUTES)) {
throw new RuntimeException("Test Failed: Manual test timed out!!");
}
}
If `await` timed out, the user didn't click the Start button.
Thus, `testStarted` becomes redundant.
test/jdk/java/awt/Robot/RobotModifierMaskTest.java line 245:
> 243: private static void createTestUI() {
> 244: String mode = isManual ? "MANUAL" : "AUTOMATED";
> 245: jFrame = new JFrame("RobotModifierMaskTest - Mode: " + mode);
Should you dispose of the instruction frame in the manual mode?
test/jdk/java/awt/Robot/RobotModifierMaskTest.java line 277:
> 275: }
> 276:
> 277: private static class CustomWindowAdapter extends WindowAdapter {
Suggestion:
private static class CloseWindowHandler extends WindowAdapter {
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14744#discussion_r1272589627
PR Review Comment: https://git.openjdk.org/jdk/pull/14744#discussion_r1272589982
PR Review Comment: https://git.openjdk.org/jdk/pull/14744#discussion_r1272598208
PR Review Comment: https://git.openjdk.org/jdk/pull/14744#discussion_r1272600122
PR Review Comment: https://git.openjdk.org/jdk/pull/14744#discussion_r1272609498
PR Review Comment: https://git.openjdk.org/jdk/pull/14744#discussion_r1272606228
More information about the client-libs-dev
mailing list