On Fri, 30 Sep 2022 20:35:21 GMT, Phil Race <p...@openjdk.org> wrote:
>> Hi there! >> JetBrains has faced with a bug on Apple M2 MacBooks when tapping (_not_ >> pressing) with two fingers on a trackpad generates wrong mouse modifiers >> (which are returned by >> [MouseEvent.getModifiersEx](https://docs.oracle.com/en/java/javase/17/docs/api/java.desktop/java/awt/event/InputEvent.html#getModifiersEx())). >> >> <s>As far as I see, OpenJDK bug tracker still doesn't contain such a bug and >> I don't have rights to create a new one, so the PR doesn't refer any id</s> >> UPD: fixed. Here is the bug link from the JetBrains own tracker: >> [JBR-4765](https://youtrack.jetbrains.com/issue/JBR-4765/Cannot-invoke-context-menu-by-two-fingers-tapping-on-MacBook-with-M2-chip). >> >> The bug is 100% reproducible on M2 MacBooks (at least under macOS 12.5). >> It's also reproducible on M1 MacBooks, but much more rarely (about 10-15% of >> reproducibility). >> >> ## Steps to reproduce >> 1. Enable `System Preferences` -> `Trackpad` -> `Tap to click` >> 2. Tap with two fingers in the following app: >> >> import javax.swing.*; >> import java.awt.event.MouseAdapter; >> import java.awt.event.MouseEvent; >> >> class MouseWindow { >> final JFrame frame; >> >> MouseWindow() { >> frame = new JFrame(); >> frame.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE); >> >> frame.setTitle("Mouse window"); >> >> frame.addMouseListener(new MouseAdapter() { >> @Override >> public void mousePressed(MouseEvent e) { >> System.out.println(e); >> } >> }); >> >> frame.pack(); >> >> frame.setSize(300, 300); >> >> frame.setVisible(true); >> } >> >> public static void main(String[] args) { >> SwingUtilities.invokeLater(MouseWindow::new); >> } >> } >> >> >> ### Expected >> Printed mouse event has `modifiersEx` is `4096` (which is >> `InputEvent.BUTTON3_DOWN_MASK`) >> >> ### Actual >> Printed mouse event has `modifiersEx` is `4352` (which is >> `InputEvent.BUTTON3_DOWN_MASK | InputEvent.META_DOWN_MASK`) >> >> ## Evaluation >> The following happens when a native mouse event reaches Java: >> 1. `NSEvent.nsToJavaModifiers(0)` called inside >> [CPlatformResponder.handleMouseEvent():81](https://github.com/openjdk/jdk/blob/5ae6bc23e857535532b59aae674e2b917bbf7284/src/java.desktop/macosx/classes/sun/lwawt/macosx/CPlatformResponder.java#L81) >> returns `0`. Earlier it always returned `4096` >> (`InputEvent.BUTTON3_DOWN_MASK`) but not in the cases of M2 tapping. For a >> trackpad press (not a tap) it still returns `4096`; >> 2. So, the `0` modifier comes into MouseEvent constructor which then [goes >> into >> MouseEvent.setOldModifiers()](https://github.com/openjdk/jdk/blob/5ae6bc23e857535532b59aae674e2b917bbf7284/src/java.desktop/share/classes/java/awt/event/MouseEvent.java#L800) >> which [initializes the field MouseEvent.modifiers to the value >> 4](https://github.com/openjdk/jdk/blob/5ae6bc23e857535532b59aae674e2b917bbf7284/src/java.desktop/share/classes/java/awt/event/MouseEvent.java#L1159) >> (it's `InputEvent.BUTTON3_MASK`); >> 3. Next, this constructed `MouseEvent` object is pushed into EDT queue. >> 4. Next, when a EDT thread pulls the event from the queue and starts to >> process it, it goes into `java.awt.LightweightDispatcher.retargetMouseEvent` >> and [creates a new MouseEvent based on the pulled >> one](https://github.com/openjdk/jdk/blob/5ae6bc23e857535532b59aae674e2b917bbf7284/src/java.desktop/share/classes/java/awt/Container.java#L4920) >> with the new value of the modifiers == `getModifiersEx() | getModifiers()`. >> From the p.2 we know, that `MouseEvent.modifiers` of the pulled event is >> `4`, so `getModifiersEx() | getModifiers()` is evaluated to `4`. >> 5. Next, the constructor of the new MouseEvent [goes into >> setNewModifiers()](https://github.com/openjdk/jdk/blob/5ae6bc23e857535532b59aae674e2b917bbf7284/src/java.desktop/share/classes/java/awt/event/MouseEvent.java#L795) >> (instead of `setOldModifiers` as in p.2) and initializes its own field >> modifiers to the value `4356` (`InputEvent.BUTTON3_MASK | >> InputEvent.BUTTON3_DOWN_MASK | InputEvent.META_DOWN_MASK`, see >> [setNewModifiers():1099](https://github.com/openjdk/jdk/blob/5ae6bc23e857535532b59aae674e2b917bbf7284/src/java.desktop/share/classes/java/awt/event/MouseEvent.java#L1099), >> >> [setNewModifiers():1129](https://github.com/openjdk/jdk/blob/5ae6bc23e857535532b59aae674e2b917bbf7284/src/java.desktop/share/classes/java/awt/event/MouseEvent.java#L1129)). >> 6. Thus, an app receives the instance of MouseEvent with `getModifiers()` == >> `4` and `getModifiersEx()` == `4352` so it thinks there is a `Command` >> keystroke. >> >> ## Fixing >> Let's set manually inside `CPlatformResponder.handleMouseEvent` a mouse >> modifier which corresponds to the pressed button. >> >> ## What about a regression test >> I've failed to write a regression test using Robot because it somehow forces >> the correct mouse modifiers (`NSEvent.nsToJavaModifiers()` correctly returns >> `InputEvent.BUTTON3_DOWN_MASK`), so I wrote a test that directly invokes >> `CPlatformResponder.handleMouseEvent` via reflection. > > src/java.desktop/macosx/classes/sun/lwawt/macosx/CPlatformResponder.java line > 83: > >> 81: int jmodifiers = NSEvent.nsToJavaModifiers(modifierFlags); >> 82: if ((jeventType == MouseEvent.MOUSE_PRESSED) && (jbuttonNumber > >> MouseEvent.NOBUTTON)) { >> 83: // 8294426: NSEvent.nsToJavaModifiers returns 0 on M2 >> MacBooks if the event is generated > > You say it returns 0, so you expect to only need to do that if jmodifiers is > 0, yet you don't check that. > Why not ? > > No way to test this on an M2 so being sure it doesn't regress something is > all I can offer here. Actually I mean that it returns 0 if you're tapping without holding any modifiers e.g. keyboard modifiers. But if you're tapping holding a keyboard modifier it will probably return only this keyboard modifier without `InputEvent.BUTTON3_DOWN_MASK`. I didn't test this case but I would expect it that looking at the implementation of the `NSEvent.nsToJavaModifiers`. ------------- PR: https://git.openjdk.org/jdk/pull/10429