xwayland: Only update keyboard modifers for XWayland's keys

KeyboardInterface is a multiplexer, it has a global state to kwin
that forwards events the single focussed window.

XWayland also forwards events to clients, but uses the keyboard interface.
It has some overloads that take a specific client, this was used for key events
but not modifiers.

The end result was not only that XWayland could miss a modifier update, but
also that wayland clients would get modifier updates out of order.
Key events must come first.

BUG: 490270

autotests/integration/x11keyread.cpp

@@ -6,6 +6,8 @@
 
  SPDX-License-Identifier: GPL-2.0-or-later
 */
+#include "KWayland/Client/keyboard.h"
+#include "KWayland/Client/seat.h"
 #include "kwin_wayland_test.h"
 
 #include "input.h"
@@ -53,6 +55,7 @@ private Q_SLOTS:
     void testSimpleLetter();
     void onlyModifier();
     void letterWithModifier();
+    void testWaylandWindowHasFocus();
 
 private:
     QList<KeyAction> recievedX11EventsForInput(const QList<KeyAction> &keyEventsIn);
@@ -91,10 +94,14 @@ void X11KeyReadTest::init()
     options->setXwaylandEavesdrops(operatingMode);
     workspace()->setActiveOutput(QPoint(640, 512));
     KWin::input()->pointer()->warp(QPoint(640, 512));
+
+    QVERIFY(Test::setupWaylandConnection(Test::AdditionalWaylandInterface::Seat));
+    QVERIFY(Test::waitForWaylandKeyboard());
 }
 
 void X11KeyReadTest::cleanup()
 {
+    Test::destroyWaylandConnection();
 }
 
 void X11KeyReadTest::testSimpleLetter()
@@ -176,6 +183,55 @@ void X11KeyReadTest::letterWithModifier()
     QCOMPARE(received, expected);
 }
 
+void X11KeyReadTest::testWaylandWindowHasFocus()
+{
+    // A wayland window should be unaffected
+    int timestamp = 0;
+
+    std::unique_ptr<KWayland::Client::Surface> surface(Test::createSurface());
+    std::unique_ptr<Test::XdgToplevel> shellSurface(Test::createXdgToplevelSurface(surface.get()));
+    std::unique_ptr<KWayland::Client::Keyboard> keyboard(Test::waylandSeat()->createKeyboard());
+    QSignalSpy modifierSpy(keyboard.get(), &KWayland::Client::Keyboard::modifiersChanged);
+    QSignalSpy keyChangedSpy(keyboard.get(), &KWayland::Client::Keyboard::keyChanged);
+
+    Window *waylandWindow = Test::renderAndWaitForShown(surface.get(), QSize(10, 10), Qt::blue);
+    QVERIFY(waylandWindow->isActive());
+    modifierSpy.wait(); // initial modifiers
+    modifierSpy.clear();
+
+    Test::keyboardKeyPressed(KEY_LEFTALT, timestamp++);
+    QVERIFY(keyChangedSpy.wait());
+    QCOMPARE(keyChangedSpy.last()[0], KEY_LEFTALT);
+    QCOMPARE(keyChangedSpy.last()[1].value<KWayland::Client::Keyboard::KeyState>(), KWayland::Client::Keyboard::KeyState::Pressed);
+
+    QCOMPARE(modifierSpy.count(), 1);
+    QCOMPARE(modifierSpy.last()[0], 8);
+    QCOMPARE(modifierSpy.last()[1], 0);
+    QCOMPARE(modifierSpy.last()[2], 0);
+    QCOMPARE(modifierSpy.last()[3], 0);
+
+    Test::keyboardKeyPressed(KEY_G, timestamp++);
+    QVERIFY(keyChangedSpy.wait());
+    QCOMPARE(keyChangedSpy.last()[0], KEY_G);
+    QCOMPARE(keyChangedSpy.last()[1].value<KWayland::Client::Keyboard::KeyState>(), KWayland::Client::Keyboard::KeyState::Pressed);
+
+    Test::keyboardKeyReleased(KEY_G, timestamp++);
+    QVERIFY(keyChangedSpy.wait());
+    QCOMPARE(keyChangedSpy.last()[0], KEY_G);
+    QCOMPARE(keyChangedSpy.last()[1].value<KWayland::Client::Keyboard::KeyState>(), KWayland::Client::Keyboard::KeyState::Released);
+
+    Test::keyboardKeyReleased(KEY_LEFTALT, timestamp++);
+    QVERIFY(keyChangedSpy.wait());
+    QCOMPARE(keyChangedSpy.last()[0], KEY_LEFTALT);
+    QCOMPARE(keyChangedSpy.last()[1].value<KWayland::Client::Keyboard::KeyState>(), KWayland::Client::Keyboard::KeyState::Released);
+
+    QCOMPARE(modifierSpy.count(), 2);
+    QCOMPARE(modifierSpy.last()[0], 0);
+    QCOMPARE(modifierSpy.last()[1], 0);
+    QCOMPARE(modifierSpy.last()[2], 0);
+    QCOMPARE(modifierSpy.last()[3], 0);
+}
+
 class X11EventRecorder :
     public
         QObject

src/wayland/keyboard.cpp

@@ -237,6 +237,15 @@ void KeyboardInterface::sendModifiers(quint32 depressed, quint32 latched, quint3
     }
 }
 
+void KeyboardInterface::sendModifiers(quint32 depressed, quint32 latched, quint32 locked, quint32 group, ClientConnection *client)
+{
+    const QList<KeyboardInterfacePrivate::Resource *> keyboards = d->keyboardsForClient(client);
+    const quint32 serial = d->seat->display()->nextSerial();
+    for (KeyboardInterfacePrivate::Resource *keyboardResource : keyboards) {
+        d->send_modifiers(keyboardResource->handle, serial, depressed, latched, locked, group);
+    }
+}
+
 void KeyboardInterface::setRepeatInfo(qint32 charactersPerSecond, qint32 delay)
 {
     d->keyRepeat.charactersPerSecond = std::max(charactersPerSecond, 0);

src/wayland/keyboard.h

@@ -57,6 +57,7 @@ public:
     void sendKey(quint32 key, KeyboardKeyState state);
     void sendKey(quint32 key, KeyboardKeyState state, ClientConnection *client);
     void sendModifiers(quint32 depressed, quint32 latched, quint32 locked, quint32 group);
+    void sendModifiers(quint32 depressed, quint32 latched, quint32 locked, quint32 group, ClientConnection *client);
 
 private:
     void setFocusedSurface(SurfaceInterface *surface, quint32 serial);

src/xwayland/xwayland.cpp

@@ -104,6 +104,7 @@ public:
                                 keyboard->sendKey(it.key(), KeyboardKeyState::Released, waylandServer()->xWaylandConnection());
                             }
                         }
+                        m_modifiers = {};
                         m_states.clear();
                     }
                 });
@@ -399,12 +400,35 @@ public:
         }
 
         auto xkb = input()->keyboard()->xkb();
+
+        keyboard->sendKey(event->nativeScanCode(), state, xwaylandClient);
+
+        bool changed = false;
+        if (m_modifiers.depressed != xkb->modifierState().depressed) {
+            m_modifiers.depressed = xkb->modifierState().depressed;
+            changed = true;
+        }
+        if (m_modifiers.latched != xkb->modifierState().latched) {
+            m_modifiers.latched = xkb->modifierState().latched;
+            changed = true;
+        }
+        if (m_modifiers.locked != xkb->modifierState().locked) {
+            m_modifiers.locked = xkb->modifierState().locked;
+            changed = true;
+        }
+        if (m_modifiers.group != xkb->currentLayout()) {
+            m_modifiers.group = xkb->currentLayout();
+            changed = true;
+        }
+        if (!changed) {
+            return;
+        }
+
         keyboard->sendModifiers(xkb->modifierState().depressed,
                                 xkb->modifierState().latched,
                                 xkb->modifierState().locked,
-                                xkb->currentLayout());
-
-        keyboard->sendKey(event->nativeScanCode(), state, xwaylandClient);
+                                xkb->currentLayout(),
+                                xwaylandClient);
     }
 
     void pointerEvent(KWin::MouseEvent *event) override
@@ -447,6 +471,13 @@ public:
     }
 
     QHash<quint32, KeyboardKeyState> m_states;
+    struct Modifiers
+    {
+        quint32 depressed = 0;
+        quint32 latched = 0;
+        quint32 locked = 0;
+        quint32 group = 0;
+    } m_modifiers;
     std::function<bool(int key, Qt::KeyboardModifiers)> m_filterKey;
     bool m_filterMouse = false;
 };