Fixed incorrect handling of extra mouse buttons in XCB platform plugin.

translateMouseButtons() was only able to correctly handle the left,
right, and mid mouse buttons. Thus, if any buttons outside of those were
pressed, the motion notify event would basically unset the mouse button
mask, leading to a release event being sent instead. Later on, when the
actual release event arrives, that button gets xor'ed into the empty
button mask generating another press, and later another motion event
will generate a release. In the end that means two press-release
sequences are sent for any extra mouse buttons.

Instead of getting the mask of the buttons currently being pressed from
the event, we need to keep track of the mask ourselves.

Task-number: QTBUG-28561
Change-Id: Iaa67e784a13d792deef8fc29dbd5456a5471a861
Reviewed-by: Oliver Wolff <oliver.wolff@digia.com>
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@digia.com>
Reviewed-by: Shawn Rutledge <shawn.rutledge@digia.com>
Reviewed-by: Rick Stockton <rickstockton@reno-computerhelp.com>
Reviewed-by: Lars Knoll <lars.knoll@digia.com>

4 files changed, 87 insertions, 59 deletions

diff --git a/src/plugins/platforms/xcb/qxcbconnection.cpp b/src/plugins/platforms/xcb/qxcbconnection.cpp
index 4c4df137a3..b4f1a278a2 100644
--- a/src/plugins/platforms/xcb/qxcbconnection.cpp
+++ b/src/plugins/platforms/xcb/qxcbconnection.cpp
@@ -257,6 +257,7 @@ QXcbConnection::QXcbConnection(QXcbNativeInterface *nativeInterface, const char
     , has_shape_extension(false)
     , has_randr_extension(false)
     , has_input_shape(false)
+    , m_buttons(0)
 {
 #ifdef XCB_USE_XLIB
     Display *dpy = XOpenDisplay(m_displayName.constData());
@@ -662,6 +663,73 @@ void QXcbConnection::handleXcbError(xcb_generic_error_t *error)
 #endif
 }
 
+static Qt::MouseButtons translateMouseButtons(int s)
+{
+    Qt::MouseButtons ret = 0;
+    if (s & XCB_BUTTON_MASK_1)
+        ret |= Qt::LeftButton;
+    if (s & XCB_BUTTON_MASK_2)
+        ret |= Qt::MidButton;
+    if (s & XCB_BUTTON_MASK_3)
+        ret |= Qt::RightButton;
+    return ret;
+}
+
+static Qt::MouseButton translateMouseButton(xcb_button_t s)
+{
+    switch (s) {
+    case 1: return Qt::LeftButton;
+    case 2: return Qt::MidButton;
+    case 3: return Qt::RightButton;
+    // Button values 4-7 were already handled as Wheel events, and won't occur here.
+    case 8: return Qt::BackButton;      // Also known as Qt::ExtraButton1
+    case 9: return Qt::ForwardButton;   // Also known as Qt::ExtraButton2
+    case 10: return Qt::ExtraButton3;
+    case 11: return Qt::ExtraButton4;
+    case 12: return Qt::ExtraButton5;
+    case 13: return Qt::ExtraButton6;
+    case 14: return Qt::ExtraButton7;
+    case 15: return Qt::ExtraButton8;
+    case 16: return Qt::ExtraButton9;
+    case 17: return Qt::ExtraButton10;
+    case 18: return Qt::ExtraButton11;
+    case 19: return Qt::ExtraButton12;
+    case 20: return Qt::ExtraButton13;
+    case 21: return Qt::ExtraButton14;
+    case 22: return Qt::ExtraButton15;
+    case 23: return Qt::ExtraButton16;
+    case 24: return Qt::ExtraButton17;
+    case 25: return Qt::ExtraButton18;
+    case 26: return Qt::ExtraButton19;
+    case 27: return Qt::ExtraButton20;
+    case 28: return Qt::ExtraButton21;
+    case 29: return Qt::ExtraButton22;
+    case 30: return Qt::ExtraButton23;
+    case 31: return Qt::ExtraButton24;
+    default: return Qt::NoButton;
+    }
+}
+
+void QXcbConnection::handleButtonPress(xcb_generic_event_t *ev)
+{
+    xcb_button_press_event_t *event = (xcb_button_press_event_t *)ev;
+
+    // the event explicitly contains the state of the three first buttons,
+    // the rest we need to manage ourselves
+    m_buttons = (m_buttons & ~0x7) | translateMouseButtons(event->state);
+    m_buttons |= translateMouseButton(event->detail);
+}
+
+void QXcbConnection::handleButtonRelease(xcb_generic_event_t *ev)
+{
+    xcb_button_release_event_t *event = (xcb_button_release_event_t *)ev;
+
+    // the event explicitly contains the state of the three first buttons,
+    // the rest we need to manage ourselves
+    m_buttons = (m_buttons & ~0x7) | translateMouseButtons(event->state);
+    m_buttons &= ~translateMouseButton(event->detail);
+}
+
 void QXcbConnection::handleXcbEvent(xcb_generic_event_t *event)
 {
 #ifdef Q_XCB_DEBUG
@@ -686,8 +754,10 @@ void QXcbConnection::handleXcbEvent(xcb_generic_event_t *event)
         case XCB_EXPOSE:
             HANDLE_PLATFORM_WINDOW_EVENT(xcb_expose_event_t, window, handleExposeEvent);
         case XCB_BUTTON_PRESS:
+            handleButtonPress(event);
             HANDLE_PLATFORM_WINDOW_EVENT(xcb_button_press_event_t, event, handleButtonPressEvent);
         case XCB_BUTTON_RELEASE:
+            handleButtonRelease(event);
             HANDLE_PLATFORM_WINDOW_EVENT(xcb_button_release_event_t, event, handleButtonReleaseEvent);
         case XCB_MOTION_NOTIFY:
             HANDLE_PLATFORM_WINDOW_EVENT(xcb_motion_notify_event_t, event, handleMotionNotifyEvent);
diff --git a/src/plugins/platforms/xcb/qxcbconnection.h b/src/plugins/platforms/xcb/qxcbconnection.h
index c67acb3218..f1e707e0cd 100644
--- a/src/plugins/platforms/xcb/qxcbconnection.h
+++ b/src/plugins/platforms/xcb/qxcbconnection.h
@@ -380,6 +380,8 @@ public:
 
     xcb_timestamp_t getTimestamp();
 
+    Qt::MouseButtons buttons() const { return m_buttons; }
+
 private slots:
     void processXcbEvents();
 
@@ -400,6 +402,8 @@ private:
     QXcbScreen* findOrCreateScreen(QList<QXcbScreen *>& newScreens, int screenNumber,
         xcb_screen_t* xcbScreen, xcb_randr_get_output_info_reply_t *output = NULL);
     void updateScreens();
+    void handleButtonPress(xcb_generic_event_t *event);
+    void handleButtonRelease(xcb_generic_event_t *event);
 
     bool m_xi2Enabled;
     int m_xi2Minor;
@@ -501,6 +505,8 @@ private:
     bool has_shape_extension;
     bool has_randr_extension;
     bool has_input_shape;
+
+    Qt::MouseButtons m_buttons;
 };
 
 #define DISPLAY_FROM_XCB(object) ((Display *)(object->connection()->xlib_display()))
diff --git a/src/plugins/platforms/xcb/qxcbwindow.cpp b/src/plugins/platforms/xcb/qxcbwindow.cpp
index 23e59f0735..05d7dc4da9 100644
--- a/src/plugins/platforms/xcb/qxcbwindow.cpp
+++ b/src/plugins/platforms/xcb/qxcbwindow.cpp
@@ -1464,53 +1464,6 @@ void QXcbWindow::handleUnmapNotifyEvent(const xcb_unmap_notify_event_t *event)
     }
 }
 
-static Qt::MouseButtons translateMouseButtons(int s)
-{
-    Qt::MouseButtons ret = 0;
-    if (s & XCB_BUTTON_MASK_1)
-        ret |= Qt::LeftButton;
-    if (s & XCB_BUTTON_MASK_2)
-        ret |= Qt::MidButton;
-    if (s & XCB_BUTTON_MASK_3)
-        ret |= Qt::RightButton;
-    return ret;
-}
-
-static Qt::MouseButton translateMouseButton(xcb_button_t s)
-{
-    switch (s) {
-    case 1: return Qt::LeftButton;
-    case 2: return Qt::MidButton;
-    case 3: return Qt::RightButton;
-    // Button values 4-7 were already handled as Wheel events, and won't occur here.
-    case 8: return Qt::BackButton;      // Also known as Qt::ExtraButton1
-    case 9: return Qt::ForwardButton;   // Also known as Qt::ExtraButton2
-    case 10: return Qt::ExtraButton3;
-    case 11: return Qt::ExtraButton4;
-    case 12: return Qt::ExtraButton5;
-    case 13: return Qt::ExtraButton6;
-    case 14: return Qt::ExtraButton7;
-    case 15: return Qt::ExtraButton8;
-    case 16: return Qt::ExtraButton9;
-    case 17: return Qt::ExtraButton10;
-    case 18: return Qt::ExtraButton11;
-    case 19: return Qt::ExtraButton12;
-    case 20: return Qt::ExtraButton13;
-    case 21: return Qt::ExtraButton14;
-    case 22: return Qt::ExtraButton15;
-    case 23: return Qt::ExtraButton16;
-    case 24: return Qt::ExtraButton17;
-    case 25: return Qt::ExtraButton18;
-    case 26: return Qt::ExtraButton19;
-    case 27: return Qt::ExtraButton20;
-    case 28: return Qt::ExtraButton21;
-    case 29: return Qt::ExtraButton22;
-    case 30: return Qt::ExtraButton23;
-    case 31: return Qt::ExtraButton24;
-    default: return Qt::NoButton;
-    }
-}
-
 void QXcbWindow::handleButtonPressEvent(const xcb_button_press_event_t *event)
 {
     updateNetWmUserTime(event->time);
@@ -1532,7 +1485,7 @@ void QXcbWindow::handleButtonPressEvent(const xcb_button_press_event_t *event)
         return;
     }
 
-    handleMouseEvent(event->detail, event->state, event->time, local, global, modifiers);
+    handleMouseEvent(event->time, local, global, modifiers);
 }
 
 void QXcbWindow::handleButtonReleaseEvent(const xcb_button_release_event_t *event)
@@ -1541,7 +1494,12 @@ void QXcbWindow::handleButtonReleaseEvent(const xcb_button_release_event_t *even
     QPoint global(event->root_x, event->root_y);
     Qt::KeyboardModifiers modifiers = connection()->keyboard()->translateModifiers(event->state);
 
-    handleMouseEvent(event->detail, event->state, event->time, local, global, modifiers);
+    if (event->detail >= 4 && event->detail <= 7) {
+        // mouse wheel, handled in handleButtonPressEvent()
+        return;
+    }
+
+    handleMouseEvent(event->time, local, global, modifiers);
 }
 
 void QXcbWindow::handleMotionNotifyEvent(const xcb_motion_notify_event_t *event)
@@ -1550,19 +1508,13 @@ void QXcbWindow::handleMotionNotifyEvent(const xcb_motion_notify_event_t *event)
     QPoint global(event->root_x, event->root_y);
     Qt::KeyboardModifiers modifiers = connection()->keyboard()->translateModifiers(event->state);
 
-    handleMouseEvent(event->detail, event->state, event->time, local, global, modifiers);
+    handleMouseEvent(event->time, local, global, modifiers);
 }
 
-void QXcbWindow::handleMouseEvent(xcb_button_t detail, uint16_t state, xcb_timestamp_t time, const QPoint &local, const QPoint &global, Qt::KeyboardModifiers modifiers)
+void QXcbWindow::handleMouseEvent(xcb_timestamp_t time, const QPoint &local, const QPoint &global, Qt::KeyboardModifiers modifiers)
 {
     connection()->setTime(time);
-
-    Qt::MouseButtons buttons = translateMouseButtons(state);
-    Qt::MouseButton button = translateMouseButton(detail);
-
-    buttons ^= button; // X event uses state *before*, Qt uses state *after*
-
-    QWindowSystemInterface::handleMouseEvent(window(), time, local, global, buttons, modifiers);
+    QWindowSystemInterface::handleMouseEvent(window(), time, local, global, connection()->buttons(), modifiers);
 }
 
 class EnterEventChecker
diff --git a/src/plugins/platforms/xcb/qxcbwindow.h b/src/plugins/platforms/xcb/qxcbwindow.h
index 07ac0e0fcb..c624841a17 100644
--- a/src/plugins/platforms/xcb/qxcbwindow.h
+++ b/src/plugins/platforms/xcb/qxcbwindow.h
@@ -134,7 +134,7 @@ public:
     void handleFocusOutEvent(const xcb_focus_out_event_t *event);
     void handlePropertyNotifyEvent(const xcb_property_notify_event_t *event);
 
-    void handleMouseEvent(xcb_button_t detail, uint16_t state, xcb_timestamp_t time, const QPoint &local, const QPoint &global, Qt::KeyboardModifiers modifiers);
+    void handleMouseEvent(xcb_timestamp_t time, const QPoint &local, const QPoint &global, Qt::KeyboardModifiers modifiers);
 
     void updateSyncRequestCounter();
     void updateNetWmUserTime(xcb_timestamp_t timestamp);