Changeset 63383 in vbox for trunk/src/VBox/Frontends/VirtualBox

Timestamp:

Aug 12, 2016 6:56:10 PM (8 years ago)

Author:

vboxsync

Message:

bugref:8151: FE/Qt: improve X11 keyboard capturing: try to fix regressions when mouse and keyboard capturing are combined on X11 hosts. We simplify keyboard capturing noticeably by always delaying grabbing the keyboard until the next keypress occurs. This lets us get rid of a few special cases in the code which were causing problems.

File:

• trunk/src/VBox/Frontends/VirtualBox/src/runtime/UIKeyboardHandler.cpp (modified) (7 diffs)

trunk/src/VBox/Frontends/VirtualBox/src/runtime/UIKeyboardHandler.cpp

@@ -14 +14 @@
  * VirtualBox OSE distribution. VirtualBox OSE is distributed in the
  * hope that it will be useful, but WITHOUT ANY WARRANTY of any kind.
+ */
+
+/*
+ * Things worth testing when changing this code:
+ *  * That automatic keyboard capture works.
+ *  * That the keyboard is captured when the mouse is.
+ *  * That the host key releases the keyboard when the keyboard is captured
+ *    but the mouse not, and both when both are.
+ *  * That the keyboard is captured when the mouse capture notification is
+ *    displayed.
+ *  * That keyboard capture works on X11 hosts when windows are dragged with
+ *    various window managers.
+ *  * That multiple machine windows do not fight for the focus on X11 hosts
+ *    (noticeable through strange modifier key and capitals behaviour).
  */
 
@@ -269 +283 @@
         return;
 
+    /* On X11 we do not grab the keyboard as soon as it is captured, but delay it until the
+     * first keypress after the capture.  We do this for several reasons.  First, when several
+     * windows are created they all try to capture the keyboard when they get the focus.  Due to
+     * the asynchronous nature of X11 the first window may only gets notified after the last is
+     * created, and there is a dance if they respond to the notifications by grabbing the keyboard
+     * and trigger new focus changes in the process.  Second, it is sometimes inconvenient to
+     * grab the keyboard immediately on focus change: some window managers set the focus then
+     * try to grab the keyboard themselves, and sulk if they fail by refusing to e.g. drag a
+     * window using its title bar.  This code uses a hack by taking (ulong)~0 as a parameter to
+     * finalise the capture. */
+
+    /* If such view exists: */
+    if (m_views.contains(uScreenId)
 #if defined(VBOX_WS_X11) && QT_VERSION >= 0x050000
-    /* Due to X11 async nature we may have lost the focus already by the time we get the focus
-     * notification, so we do a sanity check that we still have it. If we don't have the focus
-     * and grab the keyboard now that will cause focus change which we want to avoid. This change
-     * potentially leads to a loop where two windows are continually responding to outdated focus events. */
-    const xcb_get_input_focus_cookie_t xcbFocusCookie = xcb_get_input_focus(QX11Info::connection());
-    xcb_get_input_focus_reply_t *pFocusReply = xcb_get_input_focus_reply(QX11Info::connection(), xcbFocusCookie, NULL);
-    WId actualWinId = 0;
-    if (pFocusReply)
-    {
-        actualWinId = pFocusReply->focus;
-        free(pFocusReply);
-    }
-    else
-        LogRel(("GUI: UIKeyboardHandler::captureKeyboard: XCB error on acquiring focus information detected!\n"));
-    if (m_windows.value(uScreenId)->winId() != actualWinId)
-        return;
-
-    /* Delay capturing the keyboard if the mouse button is held down and the mouse is not
-     * captured to work around window managers which transfer the focus when the user
-     * clicks in the title bar and then try to grab the keyboard and sulk if they fail.
-     * If the click is inside of our views we will do the capture when it is released. */
-    const xcb_query_pointer_cookie_t xcbPointerCookie = xcb_query_pointer(QX11Info::connection(), QX11Info::appRootWindow());
-    xcb_query_pointer_reply_t *pPointerReply = xcb_query_pointer_reply(QX11Info::connection(), xcbPointerCookie, NULL);
-    if (!uisession()->isMouseCaptured() && pPointerReply && (pPointerReply->mask & XCB_KEY_BUT_MASK_BUTTON_1))
-    {
-        free(pPointerReply);
-        return;
-    }
-    free(pPointerReply);
-#endif /* VBOX_WS_X11 && QT_VERSION >= 0x050000 */
-
-    /* If such view exists: */
-    if (m_views.contains(uScreenId))
+        || ((uScreenId == (ulong)~0) && m_views.contains(m_idxDelayedKeyboardCaptureView))
+#endif
+        )
     {
 #if defined(VBOX_WS_MAC)
@@ -371 +368 @@
 # else /* QT_VERSION >= 0x050000 */
 
+        if (uScreenId != (ulong)~0)
+        {
+            m_idxDelayedKeyboardCaptureView = uScreenId;
+            return;
+        }
+        uScreenId = m_idxDelayedKeyboardCaptureView;
         /* On X11, we are using XCB stuff to grab the keyboard.
          * This stuff is a part of the active keyboard grabbing functionality.
-         * Active keyboard grabbing causes a problems on certain old window managers - a window cannot
+         * Active keyboard grabbing causes a problems on many window managers - a window cannot
          * be moved using the mouse. So we additionally grabbing the mouse as well to detect that user
          * is trying to click outside of internal window geometry. */
 
-        /* Grab the mouse button (if mouse is not captured),
-         * We do not check for failure as we do not currently implement a back-up plan. */
-        if (!uisession()->isMouseCaptured())
-            xcb_grab_button_checked(QX11Info::connection(), 0, QX11Info::appRootWindow(),
-                                    XCB_EVENT_MASK_BUTTON_PRESS, XCB_GRAB_MODE_SYNC, XCB_GRAB_MODE_ASYNC,
-                                    XCB_NONE, XCB_NONE, XCB_BUTTON_INDEX_1, XCB_MOD_MASK_ANY);
+        /* Grab the mouse button.  We do not check for failure as we do not currently implement a
+         * back-up plan. */
+        xcb_grab_button_checked(QX11Info::connection(), 0, QX11Info::appRootWindow(),
+                                XCB_EVENT_MASK_BUTTON_PRESS, XCB_GRAB_MODE_SYNC, XCB_GRAB_MODE_ASYNC,
+                                XCB_NONE, XCB_NONE, XCB_BUTTON_INDEX_1, XCB_MOD_MASK_ANY);
         /* And grab the keyboard, using XCB directly, as Qt does not report failure. */
         xcb_grab_keyboard_cookie_t xcbGrabCookie = xcb_grab_keyboard(QX11Info::connection(), false, m_views[uScreenId]->winId(),
@@ -390 +392 @@
         {
             /* Try again later: */
-            m_idxDelayedKeyboardCaptureView = uScreenId;
             free(pGrabReply);
             return;
@@ -1379 +1380 @@
             /* If we were asked to grab the keyboard previously but had to delay it
              * then try again on every key press and release event until we manage: */
-            if (m_idxDelayedKeyboardCaptureView != -1)
-                captureKeyboard(m_idxDelayedKeyboardCaptureView);
+            captureKeyboard((ulong)~0);
 
             /* Cast to XCB key-event: */
@@ -1484 +1484 @@
             }
             /* Else if the event happened outside of our view areas then release the keyboard,
-             * but set the delayed capture index so that it will be captured again if we still
-             * have the focus after the event is handled: */
+             * but capture it again (delayed) immediately.  If the event causes us to loose the
+             * focus then the delayed capture will not happen: */
             releaseKeyboard();
-            m_idxDelayedKeyboardCaptureView = uScreenId;
+            captureKeyboard(uScreenId);
             /* And re-send the event so that the window which it was meant for actually gets it: */
             xcb_allow_events_checked(QX11Info::connection(), XCB_ALLOW_REPLAY_POINTER, pButtonEvent->time);
@@ -1799 +1799 @@
                     if (!isAutoCaptureDisabled() && autoCaptureSetGlobally())
 #endif /* !VBOX_WS_WIN */
-#if defined(VBOX_WS_X11) && QT_VERSION >= 0x050000
-                        m_idxDelayedKeyboardCaptureView = uScreenId;
-#else /* !VBOX_WS_X11 || QT_VERSION < 0x050000 */
                         captureKeyboard(uScreenId);
-#endif /* !VBOX_WS_X11 || QT_VERSION < 0x050000 */
                     /* Reset the single-time disable capture flag: */
                     if (isAutoCaptureDisabled())