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;
          }

Reply via email to