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

Reply via email to