RFR: 8294426: Two fingers tap generates wrong mouse modifiers on M2 MacBooks

[Phil Race]

On Mon, 26 Sep 2022 17:36:04 GMT, Nikita Provotorov <duke at 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.

-------------

PR: https://git.openjdk.org/jdk/pull/10429