File 0001-Fix-keyboard-layout-change-notifications.p... of Package kglobalaccel

Overview Repositories Revisions Requests Users Attributes Meta

File 0001-Fix-keyboard-layout-change-notifications.patch of Package kglobalaccel (Revision 6a0e1eaa0ee6f31968c6f02a73736111)

Currently displaying revision 6a0e1eaa0ee6f31968c6f02a73736111 , Show latest

From 4d28bd4183d20a9bff4f50e88cbb47ffef0ede43 Mon Sep 17 00:00:00 2001
From: Fabian Vogt <fabian@ritter-vogt.de>
Date: Fri, 26 Oct 2018 10:01:03 +0200
Subject: [PATCH] Fix keyboard layout change notifications

Summary:
This rework fixes several issues:
- Qt wasn't informed about XCB_MAPPING_NOTIFY anymore
- With XKB enabled in Qt, X won't send XCB_MAPPING_NOTIFY anymore.
  So listen for XKB events as well.
- Install the event filter before fetching the keysym mapping to
  close a race window
- Use the old mapping for ungrabbing

BUG: 350816
BUG: 269403

Test Plan:
Ctrl-Alt-Y global shortcut works even when doing
"setxkbmap us; kglobalaccel5 & sleep 1; setxkbmap de;" while it did not
before. On some systems, this race happened on every login, now it works
reliably.

Reviewers: #frameworks, #plasma, romangg

Reviewed By: #plasma, romangg

Subscribers: romangg, ngraham, anthonyfieroni, kde-frameworks-devel

Tags: #frameworks

Differential Revision: https://phabricator.kde.org/D16434
---
 CMakeLists.txt                               |  2 +-
 src/runtime/plugins/CMakeLists.txt           |  2 +-
 src/runtime/plugins/xcb/CMakeLists.txt       |  1 +
 src/runtime/plugins/xcb/kglobalaccel_x11.cpp | 76 ++++++++++++++------
 src/runtime/plugins/xcb/kglobalaccel_x11.h   |  1 +
 5 files changed, 59 insertions(+), 23 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 85c5913..c12fb4d 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -49,7 +49,7 @@ find_package(KF5WindowSystem ${KF5_DEP_VERSION} REQUIRED)
 
 # no X11 stuff on mac
 if (NOT APPLE)
-    find_package(XCB MODULE COMPONENTS XCB KEYSYMS XTEST)
+    find_package(XCB MODULE COMPONENTS XCB KEYSYMS XTEST XKB)
     set_package_properties(XCB PROPERTIES DESCRIPTION "X protocol C-language Binding"
                        URL "http://xcb.freedesktop.org"
                        TYPE OPTIONAL
diff --git a/src/runtime/plugins/CMakeLists.txt b/src/runtime/plugins/CMakeLists.txt
index 316b9bf..29e45d4 100644
--- a/src/runtime/plugins/CMakeLists.txt
+++ b/src/runtime/plugins/CMakeLists.txt
@@ -1,4 +1,4 @@
-if (${XCB_XCB_FOUND} AND ${XCB_KEYSYMS_FOUND})
+if (${XCB_XCB_FOUND} AND ${XCB_KEYSYMS_FOUND} AND ${XCB_XKB_FOUND})
     add_subdirectory(xcb)
 endif()
 # if (APPLE)
diff --git a/src/runtime/plugins/xcb/CMakeLists.txt b/src/runtime/plugins/xcb/CMakeLists.txt
index 45bf4dc..b76477f 100644
--- a/src/runtime/plugins/xcb/CMakeLists.txt
+++ b/src/runtime/plugins/xcb/CMakeLists.txt
@@ -8,6 +8,7 @@ target_link_libraries(KF5GlobalAccelPrivateXcb
     KF5GlobalAccelPrivate
     XCB::XCB
     XCB::KEYSYMS
+    XCB::XKB
 )
 
 install(
diff --git a/src/runtime/plugins/xcb/kglobalaccel_x11.cpp b/src/runtime/plugins/xcb/kglobalaccel_x11.cpp
index 9b75ec4..cdee881 100644
--- a/src/runtime/plugins/xcb/kglobalaccel_x11.cpp
+++ b/src/runtime/plugins/xcb/kglobalaccel_x11.cpp
@@ -33,9 +33,13 @@
 #include <X11/keysym.h>
 
 // xcb
+
+// It uses "explicit" as a variable name, which is not allowed in C++
+#define explicit xcb_explicit
 #include <xcb/xcb.h>
 #include <xcb/xcb_keysyms.h>
-
+#include <xcb/xkb.h>
+#undef explicit
 
 // g_keyModMaskXAccel
 //	mask of modifiers which can be used in shortcuts
@@ -63,11 +67,16 @@ static void calculateGrabMasks()
 KGlobalAccelImpl::KGlobalAccelImpl(QObject *parent)
     : KGlobalAccelInterface(parent)
     , m_keySymbols(nullptr)
+    , m_xkb_first_event(0)
 {
+	Q_ASSERT(QX11Info::connection());
+
+	const xcb_query_extension_reply_t *reply = xcb_get_extension_data(QX11Info::connection(), &xcb_xkb_id);
+	if (reply && reply->present) {
+		m_xkb_first_event = reply->first_event;
+	}
+
 	calculateGrabMasks();
-    if (QX11Info::isPlatformX11()) {
-        m_keySymbols = xcb_key_symbols_alloc(QX11Info::connection());
-    }
 }
 
 KGlobalAccelImpl::~KGlobalAccelImpl()
@@ -87,8 +96,12 @@ bool KGlobalAccelImpl::grabKey( int keyQt, bool grab )
     }
 
     if (!m_keySymbols) {
-        return false;
+        m_keySymbols = xcb_key_symbols_alloc(QX11Info::connection());
+        if (!m_keySymbols) {
+            return false;
+        }
     }
+
 	if( !keyQt ) {
         qCDebug(KGLOBALACCELD) << "Tried to grab key with null code.";
 		return false;
@@ -192,30 +205,45 @@ bool KGlobalAccelImpl::nativeEventFilter(const QByteArray &eventType, void *mess
     }
     xcb_generic_event_t *event = reinterpret_cast<xcb_generic_event_t*>(message);
     const uint8_t responseType = event->response_type & ~0x80;
-    switch (responseType) {
-        case XCB_MAPPING_NOTIFY:
-            qCDebug(KGLOBALACCELD) << "Got XMappingNotify event";
-            xcb_refresh_keyboard_mapping(m_keySymbols, reinterpret_cast<xcb_mapping_notify_event_t*>(event));
-            x11MappingNotify();
-            return true;
-
-        case XCB_KEY_PRESS:
+    if (responseType == XCB_MAPPING_NOTIFY) {
+        x11MappingNotify();
+
+        // Make sure to let Qt handle it as well
+        return false;
+    } else if (responseType == XCB_KEY_PRESS) {
 #ifdef KDEDGLOBALACCEL_TRACE
-            qCDebug(KGLOBALACCELD) << "Got XKeyPress event";
+        qCDebug(KGLOBALACCELD) << "Got XKeyPress event";
 #endif
-            return x11KeyPress(reinterpret_cast<xcb_key_press_event_t*>(event));
+        return x11KeyPress(reinterpret_cast<xcb_key_press_event_t*>(event));
+    } else if (m_xkb_first_event && responseType == m_xkb_first_event) {
+        const uint8_t xkbEvent = event->pad0;
+        switch (xkbEvent) {
+            case XCB_XKB_MAP_NOTIFY:
+                x11MappingNotify();
+                break;
+            case XCB_XKB_NEW_KEYBOARD_NOTIFY: {
+                const xcb_xkb_new_keyboard_notify_event_t *ev =
+                    reinterpret_cast<xcb_xkb_new_keyboard_notify_event_t*>(event);
+                if (ev->changed & XCB_XKB_NKN_DETAIL_KEYCODES)
+                    x11MappingNotify();
+                break;
+            }
+            default:
+                break;
+        }
 
-        default:
-            // We get all XEvents. Just ignore them.
-            return false;
+        // Make sure to let Qt handle it as well
+        return false;
+    } else {
+        // We get all XEvents. Just ignore them.
+        return false;
     }
-
-    Q_ASSERT(false);
-    return false;
 }
 
 void KGlobalAccelImpl::x11MappingNotify()
 {
+	qCDebug(KGLOBALACCELD) << "Got XMappingNotify event";
+
 	// Maybe the X modifier map has been changed.
 	// uint oldKeyModMaskXAccel = g_keyModMaskXAccel;
 	// uint oldKeyModMaskXOnOrOff = g_keyModMaskXOnOrOff;
@@ -226,6 +254,12 @@ void KGlobalAccelImpl::x11MappingNotify()
 	// different keycodes.
 	ungrabKeys();
 
+	if (m_keySymbols) {
+		// Force reloading of the keySym mapping
+		xcb_key_symbols_free(m_keySymbols);
+		m_keySymbols = nullptr;
+	}
+
 	KKeyServer::initializeMods();
 	calculateGrabMasks();
 
diff --git a/src/runtime/plugins/xcb/kglobalaccel_x11.h b/src/runtime/plugins/xcb/kglobalaccel_x11.h
index 4011719..1ed75d1 100644
--- a/src/runtime/plugins/xcb/kglobalaccel_x11.h
+++ b/src/runtime/plugins/xcb/kglobalaccel_x11.h
@@ -77,6 +77,7 @@ private:
         bool x11KeyPress(xcb_key_press_event_t *event);
 	
     xcb_key_symbols_t *m_keySymbols;
+    uint8_t m_xkb_first_event;
 };
 
 #endif // _KGLOBALACCEL_X11_H
-- 
2.19.0

Places

File 0001-Fix-keyboard-layout-change-notifications.patch of Package kglobalaccel (Revision 6a0e1eaa0ee6f31968c6f02a73736111)

Places