<AWT Dev> [9] Review request for 6624085: Fourth mouse button (wheel) is treated like second button - isPopupTrigger returns true

[Anthony Petrov]

The fix looks great to me. +1.

--
best regards,
Anthony

On 8/27/2014 8:31 PM, Alexander Zvegintsev wrote:
> Hello Alex,
>
> You are welcome and I am glad to see your contribution to OpenJDK project.
>
> Your fix looks good to me, it resolves JDK-6624085 issue too, so let it
> be fixed under this bug ID.
> As I can see in jdk9-dev mailing list OCA has been signed by you, so no
> further action is required from your side at this moment.
>
> But we still need a second reviewer to proceed (I've uploaded a webrev
> for convenience [1]).
>
> [1] http://cr.openjdk.java.net/~azvegint/oca/6624085/
>
> Thanks,
>
> Alexander.
>
> On 08/25/2014 11:11 PM, Alex Henrie wrote:
>> With this patch applied, only clicks on the mouse's right button and not
>> clicks to the mouse's forward or back buttons trigger a context menu
>> (MouseEvent.isPopupTrigger() == true).
>>
>> Class XWindow tried to map physical mouse buttons to logical mouse
>> buttons, but this unnecessary because that mapping is already handled
>> automatically in Xlib:
>> http://www.x.org/archive/X11R7.5/doc/man/man3/XKeyEvent.3.html
>>
>> Also, XGetPointerMapping returns the total number of physical mouse
>> buttons (16 on my computer), not the value of a map entry:
>> http://www.x.org/archive/X11R7.5/doc/man/man3/XGetPointerMapping.3.html
>>
>> This JDK bug has to be fixed before
>> https://netbeans.org/bugzilla/show_bug.cgi?id=198885 can be fixed.
>>
>> This patch will probably also fix
>> https://bugs.openjdk.java.net/browse/JDK-6624085
>>
>> A test program is attached.
>>
>> This is my first time contributing code to OpenJDK, so if I've done
>> something wrong, please be nice ;-)
>>
>> -Alex
>>
>> diff --git a/src/java.desktop/unix/classes/sun/awt/X11/XWindow.java
>> b/src/java.desktop/unix/classes/sun/awt/X11/XWindow.java
>> --- a/src/java.desktop/unix/classes/sun/awt/X11/XWindow.java
>> +++ b/src/java.desktop/unix/classes/sun/awt/X11/XWindow.java
>> @@ -50,17 +50,16 @@ class XWindow extends XBaseWindow implem
>>       private static final PlatformLogger focusLog =
>> PlatformLogger.getLogger("sun.awt.X11.focus.XWindow");
>>       private static PlatformLogger keyEventLog =
>> PlatformLogger.getLogger("sun.awt.X11.kye.XWindow");
>>     /* If a motion comes in while a multi-click is pending,
>>      * allow a smudge factor so that moving the mouse by a small
>>      * amount does not wipe out the multi-click state variables.
>>      */
>>       private final static int AWT_MULTICLICK_SMUDGE = 4;
>>       // ButtonXXX events stuff
>> -    static int rbutton = 0;
>>       static int lastX = 0, lastY = 0;
>>       static long lastTime = 0;
>>       static long lastButton = 0;
>>       static WeakReference<XWindow> lastWindowRef = null;
>>       static int clickCount = 0;
>>       // used to check if we need to re-create surfaceData.
>>       int oldWidth = -1;
>> @@ -627,33 +626,16 @@ class XWindow extends XBaseWindow implem
>>               res |= XToolkit.metaMask;
>>           }
>>           if ((mods & (InputEvent.ALT_GRAPH_DOWN_MASK |
>> InputEvent.ALT_GRAPH_MASK)) != 0) {
>>               res |= XToolkit.modeSwitchMask;
>>           }
>>           return res;
>>       }
>> -    /**
>> -     * Returns true if this event is disabled and shouldn't be passed
>> to Java.
>> -     * Default implementation returns false for all events.
>> -     */
>> -    static int getRightButtonNumber() {
>> -        if (rbutton == 0) { // not initialized yet
>> -            XToolkit.awtLock();
>> -            try {
>> -                rbutton =
>> XlibWrapper.XGetPointerMapping(XToolkit.getDisplay(),
>> XlibWrapper.ibuffer, 3);
>> -            }
>> -            finally {
>> -                XToolkit.awtUnlock();
>> -            }
>> -        }
>> -        return rbutton;
>> -    }
>> -
>>       static int getMouseMovementSmudge() {
>>           //TODO: It's possible to read corresponding settings
>>           return AWT_MULTICLICK_SMUDGE;
>>       }
>>       public void handleButtonPressRelease(XEvent xev) {
>>           super.handleButtonPressRelease(xev);
>>           XButtonEvent xbe = xev.get_xbutton();
>> @@ -711,21 +693,17 @@ class XWindow extends XBaseWindow implem
>>                   lastY = y;
>>               }
>>               lastTime = when;
>>               /*
>>                  Check for popup trigger !!
>>               */
>> -            if (lbutton == getRightButtonNumber() || lbutton > 2) {
>> -                popupTrigger = true;
>> -            } else {
>> -                popupTrigger = false;
>> -            }
>> +            popupTrigger = (lbutton == 3);
>>           }
>>           button = XConstants.buttons[lbutton - 1];
>>           // 4 and 5 buttons are usually considered assigned to a
>> first wheel
>>           if (lbutton == XConstants.buttons[3] ||
>>               lbutton == XConstants.buttons[4]) {
>>               wheel_mouse = true;
>>           }
>