wayland: Move XWayland key forwarding into a filter

We optionally send some keys to xwayland through the filter when no x11
client has focus. This allows shortcut handling in X11 apps to work.

When kwin is grabbing keys we don't necessarily want X11 to sniff these
keys as things can get out of sync. A key place is the tabbox. The X11
client sill has focus, but xwayland is not active. This means we pass
tab keys to X which then go to application incorrectly.

Part of this patch changes the tabbox filter to not intercept the alt
key release event. This ensures xwayland's concept of pressed modifiers
stays in sync.

BUG: 484992

autotests/integration/x11keyread.cpp

@@ -56,6 +56,7 @@ private Q_SLOTS:
     void onlyModifier();
     void letterWithModifier();
     void testWaylandWindowHasFocus();
+    void tabBox();
 
 private:
     QList<KeyAction> recievedX11EventsForInput(const QList<KeyAction> &keyEventsIn);
@@ -232,6 +233,41 @@ void X11KeyReadTest::testWaylandWindowHasFocus()
     QCOMPARE(modifierSpy.last()[3], 0);
 }
 
+void X11KeyReadTest::tabBox()
+{
+    // Note this test will fail if you forget to use dbus-run-session!
+#if KWIN_BUILD_TABBOX
+    QList<KeyAction> keyEvents = {
+        {State::Press, KEY_LEFTALT},
+        {State::Press, KEY_TAB},
+        {State::Release, KEY_TAB},
+        {State::Press, KEY_TAB},
+        {State::Release, KEY_TAB},
+        {State::Release, KEY_LEFTALT},
+    };
+    auto received = recievedX11EventsForInput(keyEvents);
+
+    QList<KeyAction> expected;
+    QFETCH_GLOBAL(XwaylandEavesdropsMode, operatingMode);
+    switch (operatingMode) {
+    case XwaylandEavesdropsMode::None:
+        expected = {};
+        break;
+    // even though tab is a regular key whilst holding alt, the tab switcher should be grabbing
+    case XwaylandEavesdropsMode::AllKeysWithModifier:
+    case XwaylandEavesdropsMode::NonCharacterKeys:
+    case XwaylandEavesdropsMode::All:
+        expected = {
+            {State::Press, KEY_LEFTALT},
+            {State::Release, KEY_LEFTALT},
+        };
+        break;
+    }
+
+    QCOMPARE(received, expected);
+#endif
+}
+
 class X11EventRecorder :
     public
         QObject

src/input.cpp

@@ -1666,6 +1666,7 @@ public:
             workspace()->tabbox()->keyPress(event->modifiers() | event->key());
         } else if (static_cast<KeyEvent *>(event)->modifiersRelevantForGlobalShortcuts() == Qt::NoModifier) {
             workspace()->tabbox()->modifiersReleased();
+            return false;
         }
         return true;
     }

src/input.h

@@ -381,6 +381,7 @@ enum Order {
     Popup,
     Decoration,
     WindowAction,
+    XWayland,
     InternalWindow,
     InputMethod,
     Forward,

src/xwayland/xwayland.cpp

@@ -19,7 +19,6 @@
 #include "xwldrophandler.h"
 
 #include "core/output.h"
-#include "input_event_spy.h"
 #include "keyboard_input.h"
 #include "main_wayland.h"
 #include "utils/common.h"
@@ -82,32 +81,32 @@ bool XrandrEventFilter::event(xcb_generic_event_t *event)
     return false;
 }
 
-class XwaylandInputSpy : public QObject, public KWin::InputEventSpy
+class XwaylandInputFilter : public QObject, public KWin::InputEventFilter
 {
 public:
-    XwaylandInputSpy()
+    XwaylandInputFilter()
+        : KWin::InputEventFilter(InputFilterOrder::XWayland)
     {
         connect(waylandServer()->seat(), &SeatInterface::focusedKeyboardSurfaceAboutToChange,
                 this, [this](SurfaceInterface *newSurface) {
-                    auto keyboard = waylandServer()->seat()->keyboard();
-                    if (!newSurface) {
-                        return;
-                    }
+            auto keyboard = waylandServer()->seat()->keyboard();
+            if (!newSurface) {
+                return;
+            }
 
-                    if (waylandServer()->xWaylandConnection() == newSurface->client()) {
-                        // Since this is a spy but the keyboard interface gets its normal sendKey calls through filters,
-                        // there can be a mismatch in both states.
-                        // This loop makes sure all key press events are reset before we switch back to the
-                        // Xwayland client and the state is correctly restored.
-                        for (auto it = m_states.constBegin(); it != m_states.constEnd(); ++it) {
-                            if (it.value() == KeyboardKeyState::Pressed) {
-                                keyboard->sendKey(it.key(), KeyboardKeyState::Released, waylandServer()->xWaylandConnection());
-                            }
-                        }
-                        m_modifiers = {};
-                        m_states.clear();
+            if (waylandServer()->xWaylandConnection() == newSurface->client()) {
+                // Since this is in the filter chain some key events may have been filtered out
+                // This loop makes sure all key press events are reset before we switch back to the
+                // Xwayland client and the state is correctly restored.
+                for (auto it = m_states.constBegin(); it != m_states.constEnd(); ++it) {
+                    if (it.value() == KeyboardKeyState::Pressed) {
+                        keyboard->sendKey(it.key(), KeyboardKeyState::Released, waylandServer()->xWaylandConnection());
                     }
-                });
+                }
+                m_modifiers = {};
+                m_states.clear();
+            }
+        });
     }
 
     void setMode(XwaylandEavesdropsMode mode, bool eavesdropsMouse)
@@ -368,35 +367,33 @@ public:
         }
     }
 
-    void keyEvent(KWin::KeyEvent *event) override
+    bool keyEvent(KWin::KeyEvent *event) override
     {
+        ClientConnection *xwaylandClient = waylandServer()->xWaylandConnection();
+        if (!xwaylandClient) {
+            return false;
+        }
         if (event->isAutoRepeat()) {
-            return;
+            return false;
         }
 
-        Window *window = workspace()->activeWindow();
-        if (!m_filterKey || !m_filterKey(event->key(), event->modifiers()) || (window && window->isLockScreen())) {
-            return;
+        if (!m_filterKey || !m_filterKey(event->key(), event->modifiers())) {
+            return false;
         }
 
         auto keyboard = waylandServer()->seat()->keyboard();
         auto surface = keyboard->focusedSurface();
-        ClientConnection *xwaylandClient = waylandServer()->xWaylandConnection();
-
-        if (!xwaylandClient) {
-            return;
-        }
 
         if (surface) {
             ClientConnection *client = surface->client();
             if (xwaylandClient == client) {
-                return;
+                return false;
             }
         }
 
         KeyboardKeyState state{event->type() == QEvent::KeyPress};
         if (!updateKey(event->nativeScanCode(), state)) {
-            return;
+            return false;
         }
 
         auto xkb = input()->keyboard()->xkb();
@@ -421,7 +418,7 @@ public:
             changed = true;
         }
         if (!changed) {
-            return;
+            return false;
         }
 
         keyboard->sendModifiers(xkb->modifierState().depressed,
@@ -429,31 +426,36 @@ public:
                                 xkb->modifierState().locked,
                                 xkb->currentLayout(),
                                 xwaylandClient);
+        return false;
     }
 
-    void pointerEvent(KWin::MouseEvent *event) override
+    bool pointerEvent(KWin::MouseEvent *event, quint32 nativeButton) override
     {
-        Window *window = workspace()->activeWindow();
-        if (!m_filterMouse || (window && window->isLockScreen())) {
-            return;
+
+        ClientConnection *xwaylandClient = waylandServer()->xWaylandConnection();
+        if (!xwaylandClient) {
+            return false;
+        }
+        if (!m_filterMouse) {
+            return false;
         }
         if (event->type() != QEvent::MouseButtonPress && event->type() != QEvent::MouseButtonRelease) {
-            return;
+            return false;
         }
 
         auto pointer = waylandServer()->seat()->pointer();
         auto surface = pointer->focusedSurface();
-        ClientConnection *xwaylandClient = waylandServer()->xWaylandConnection();
 
         if (surface) {
             ClientConnection *client = surface->client();
             if (xwaylandClient && xwaylandClient == client) {
-                return;
+                return false;
             }
         }
 
         PointerButtonState state{event->type() == QEvent::MouseButtonPress};
         pointer->sendButton(event->nativeButton(), state, xwaylandClient);
+        return false;
     }
 
     bool updateKey(quint32 key, KeyboardKeyState state)
@@ -583,7 +585,7 @@ void Xwayland::handleXwaylandFinished()
     m_compositingManagerSelectionOwner.reset();
     m_windowManagerSelectionOwner.reset();
 
-    m_inputSpy.reset();
+    m_inputFilter.reset();
     disconnect(options, &Options::xwaylandEavesdropsChanged, this, &Xwayland::refreshEavesdropping);
     disconnect(options, &Options::xwaylandEavesdropsMouseChanged, this, &Xwayland::refreshEavesdropping);
 
@@ -632,20 +634,19 @@ void Xwayland::refreshEavesdropping()
     }
 
     const bool enabled = options->xwaylandEavesdrops() != None;
-    if (enabled == bool(m_inputSpy)) {
-        if (m_inputSpy) {
-            m_inputSpy->setMode(options->xwaylandEavesdrops(), options->xwaylandEavesdropsMouse());
+    if (enabled == bool(m_inputFilter)) {
+        if (m_inputFilter) {
+            m_inputFilter->setMode(options->xwaylandEavesdrops(), options->xwaylandEavesdropsMouse());
         }
         return;
     }
 
     if (enabled) {
-        m_inputSpy = std::make_unique<XwaylandInputSpy>();
-        input()->installInputEventSpy(m_inputSpy.get());
-        m_inputSpy->setMode(options->xwaylandEavesdrops(), options->xwaylandEavesdropsMouse());
+        m_inputFilter = std::make_unique<XwaylandInputFilter>();
+        m_inputFilter->setMode(options->xwaylandEavesdrops(), options->xwaylandEavesdropsMouse());
+        input()->installInputEventFilter(m_inputFilter.get());
     } else {
-        input()->uninstallInputEventSpy(m_inputSpy.get());
-        m_inputSpy.reset();
+        m_inputFilter.reset();
     }
 }
 

src/xwayland/xwayland.h

@@ -32,7 +32,7 @@ class Application;
 namespace Xwl
 {
 class XrandrEventFilter;
-class XwaylandInputSpy;
+class XwaylandInputFilter;
 class XwaylandLauncher;
 class DataBridge;
 
@@ -92,7 +92,7 @@ private:
 
     XrandrEventFilter *m_xrandrEventsFilter = nullptr;
     XwaylandLauncher *m_launcher;
-    std::unique_ptr<XwaylandInputSpy> m_inputSpy;
+    std::unique_ptr<XwaylandInputFilter> m_inputFilter;
 
     Q_DISABLE_COPY(Xwayland)
 };