Send correct modifier state for non-windowsystem key events

In the wl_keyboard protocol, key events and modifier state is sent
separately. Currently, the modifier state is maintained by a
QWindowSystemEventHandler that filters low-level QPA events. This does
not work for key events that are injected from other sources, such as
virtual keyboards, or remote UI connections.

This change does not try to fix the architectural issue. It is just a
workaround that detects when we try to send a key event with a modifier
state that does not match the previous modifier state sent through
wl_keyboard. In that case we send a wl_keyboard.modifiers() event that
matches the modifiers of the key event.

This is somewhat fragile, and will probably cause weirdness if native
key events are interleaved with injected events, but it should behave
better than the previous situation.

Change-Id: Ic80198e7728edacfdcaf426bae54508a6576e8cd
Reviewed-by: Eskil Abrahamsen Blomfeldt <eskil.abrahamsen-blomfeldt@qt.io>

4 files changed, 60 insertions, 2 deletions

diff --git a/src/compositor/compositor_api/qwaylandkeyboard.cpp b/src/compositor/compositor_api/qwaylandkeyboard.cpp
index 223f1973..aee9e7e7 100644
--- a/src/compositor/compositor_api/qwaylandkeyboard.cpp
+++ b/src/compositor/compositor_api/qwaylandkeyboard.cpp
@@ -39,11 +39,13 @@
 #include <QtCore/QFile>
 #include <QtCore/QStandardPaths>
 
+#include <QKeyEvent>
 #include <fcntl.h>
 #include <unistd.h>
 #if QT_CONFIG(xkbcommon)
 #include <sys/mman.h>
 #include <sys/types.h>
+#include <xkbcommon/xkbcommon-names.h>
 #endif
 
 QT_BEGIN_NAMESPACE
@@ -192,6 +194,10 @@ void QWaylandKeyboardPrivate::maybeUpdateXkbScanCodeTable()
                     scanCodesByQtKey->insert({layout, qtKey}, keycode);
             }
         }, &scanCodesByQtKey);
+
+        shiftIndex = xkb_keymap_mod_get_index(keymap, XKB_MOD_NAME_SHIFT);
+        controlIndex = xkb_keymap_mod_get_index(keymap, XKB_MOD_NAME_CTRL);
+        altIndex = xkb_keymap_mod_get_index(keymap, XKB_MOD_NAME_ALT);
     }
 }
 #endif
@@ -223,6 +229,15 @@ void QWaylandKeyboardPrivate::updateModifierState(uint code, uint32_t state)
     if (focusResource) {
         send_modifiers(focusResource->handle, compositor()->nextSerial(), modsDepressed,
                        modsLatched, modsLocked, group);
+
+        Qt::KeyboardModifiers currentState = Qt::NoModifier;
+        if (xkb_state_mod_index_is_active(xkbState(), shiftIndex, XKB_STATE_MODS_EFFECTIVE) == 1)
+            currentState |= Qt::ShiftModifier;
+        if (xkb_state_mod_index_is_active(xkbState(), controlIndex, XKB_STATE_MODS_EFFECTIVE) == 1)
+            currentState |= Qt::ControlModifier;
+        if (xkb_state_mod_index_is_active(xkbState(), altIndex, XKB_STATE_MODS_EFFECTIVE) == 1)
+            currentState |= Qt::AltModifier;
+        currentModifierState = currentState;
     }
 #else
     Q_UNUSED(code);
@@ -498,6 +513,39 @@ void QWaylandKeyboard::sendKeyReleaseEvent(uint code)
     d->sendKeyEvent(code, WL_KEYBOARD_KEY_STATE_RELEASED);
 }
 
+void QWaylandKeyboard::checkAndRepairModifierState(QKeyEvent *ke)
+{
+#if QT_CONFIG(xkbcommon)
+    Q_D(QWaylandKeyboard);
+    if (ke->modifiers() != d->currentModifierState) {
+        if (d->focusResource && ke->key() != Qt::Key_Shift
+                && ke->key() != Qt::Key_Control && ke->key() != Qt::Key_Alt) {
+            // Only repair the state for non-modifier keys
+            // ### slightly awkward because the standard modifier handling
+            // is done by QtWayland::WindowSystemEventHandler after the
+            // key event is delivered
+            uint32_t mods = 0;
+
+            if (d->shiftIndex == 0 && d->controlIndex == 0)
+                d->maybeUpdateXkbScanCodeTable();
+
+            if (ke->modifiers() & Qt::ShiftModifier)
+                mods |= 1 << d->shiftIndex;
+            if (ke->modifiers() & Qt::ControlModifier)
+                mods |= 1 << d->controlIndex;
+            if (ke->modifiers() & Qt::AltModifier)
+                mods |= 1 << d->altIndex;
+            qCDebug(qLcWaylandCompositor) << "Keyboard modifier state mismatch detected for event" << ke << "state:" << d->currentModifierState << "repaired:" << Qt::hex << mods;
+            d->send_modifiers(d->focusResource->handle, compositor()->nextSerial(), mods,
+                    0, 0, d->group);
+            d->currentModifierState = ke->modifiers();
+        }
+    }
+#else
+    Q_UNUSED(ke);
+#endif
+}
+
 /*!
  * Returns the current repeat rate.
  */
diff --git a/src/compositor/compositor_api/qwaylandkeyboard.h b/src/compositor/compositor_api/qwaylandkeyboard.h
index 834260e3..9f7287a2 100644
--- a/src/compositor/compositor_api/qwaylandkeyboard.h
+++ b/src/compositor/compositor_api/qwaylandkeyboard.h
@@ -67,6 +67,8 @@ public:
     virtual void sendKeyPressEvent(uint code);
     virtual void sendKeyReleaseEvent(uint code);
 
+    void checkAndRepairModifierState(QKeyEvent *ke);
+
     QWaylandSurface *focus() const;
     QWaylandClient *focusClient() const;
 
diff --git a/src/compositor/compositor_api/qwaylandkeyboard_p.h b/src/compositor/compositor_api/qwaylandkeyboard_p.h
index de962e40..6ae0b441 100644
--- a/src/compositor/compositor_api/qwaylandkeyboard_p.h
+++ b/src/compositor/compositor_api/qwaylandkeyboard_p.h
@@ -119,6 +119,12 @@ private:
     uint32_t modsLocked = 0;
     uint32_t group = 0;
 
+    uint32_t shiftIndex = 0;
+    uint32_t controlIndex = 0;
+    uint32_t altIndex = 0;
+
+    Qt::KeyboardModifiers currentModifierState;
+
     bool pendingKeymap = false;
 #if QT_CONFIG(xkbcommon)
     size_t keymap_size;
diff --git a/src/compositor/compositor_api/qwaylandseat.cpp b/src/compositor/compositor_api/qwaylandseat.cpp
index 54538d1d..6c080b82 100644
--- a/src/compositor/compositor_api/qwaylandseat.cpp
+++ b/src/compositor/compositor_api/qwaylandseat.cpp
@@ -517,10 +517,12 @@ void QWaylandSeat::sendFullKeyEvent(QKeyEvent *event)
             return;
         }
 
-        if (event->type() == QEvent::KeyPress)
+        if (event->type() == QEvent::KeyPress) {
+            d->keyboard->checkAndRepairModifierState(event);
             d->keyboard->sendKeyPressEvent(scanCode);
-        else if (event->type() == QEvent::KeyRelease)
+        } else if (event->type() == QEvent::KeyRelease) {
             d->keyboard->sendKeyReleaseEvent(scanCode);
+        }
     }
 }