On Mon, 17 Jul 2023 19:09:29 GMT, Harshitha Onkar <hon...@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 @mrserb > Just to clarify an idea of this change. We would like to maintain the "flag" > keys pressed by the user and maintain the list of current flags. So if the > user will press SHIFT key, we will use that modifier for all next keys until > shift will be released. That is correct. > It looks similar to how the mouseMove is implemented in CRobot, where we > maintain the mouseLastX/mouseLastY, and we ignore the current location of the > mouse for clicks, but use the latest saved value. > Probably we can do the same on the java side in this patch? it will simplify > the native code a bit. Yes, the code for mouseEvent looks similar to what we are trying to achieve for keyEvent but there are a few differences too- - `mouseEvent(int lastX, int lastY, int buttonsState, boolean isButtonsDownState, boolean isMouseMove)`, lastX & lastY are already part of the parameters being sent from Java to native side. - `keyEvent(int javaKeyCode, boolean keydown)` - Are you suggesting that a modifier flag state be sent from Java to native side? This would involve changing the present native `keyEvent` signature and adding code to native side to evaluate the new modifier flag state from Java and then map it to the corresponding [kCGEventFlagMask](https://developer.apple.com/documentation/coregraphics/cgeventflags/kcgeventflagmaskalphashift?language=objc). Having everything on native side seems to be easier because with the present fix, the equivalent modifier flag mask is obtained using javaKeyCode + key state and a quick `NSDictionary modifierKeyToMaskMap` lookup without the need of adding or changing keyEvent signature. > BTW do we need to set all this modifiers for the mouse clicks as well? I don't think so as the issue (details below) occurs only when flagState is obtained using - `CGEventSourceFlagsState(kCGEventSourceStateHIDSystemState)` which was the case for keyEvents. **Issue:** _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._ ------------- PR Comment: https://git.openjdk.org/jdk/pull/14744#issuecomment-1639050801
