From 8fd4476ff1848ce7ce4d3573224fc4893ae8339d Mon Sep 17 00:00:00 2001 From: David Edmundson Date: Tue, 9 Jul 2024 12:01:41 +0100 Subject: [PATCH] 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 | 36 ++++++++++ src/input.cpp | 1 + src/input.h | 1 + src/xwayland/xwayland.cpp | 99 ++++++++++++++-------------- src/xwayland/xwayland.h | 4 +- 5 files changed, 90 insertions(+), 51 deletions(-) diff --git a/autotests/integration/x11keyread.cpp b/autotests/integration/x11keyread.cpp index cfb0b3e089..597f217012 100644 --- a/autotests/integration/x11keyread.cpp +++ b/autotests/integration/x11keyread.cpp @@ -56,6 +56,7 @@ private Q_SLOTS: void onlyModifier(); void letterWithModifier(); void testWaylandWindowHasFocus(); + void tabBox(); private: QList recievedX11EventsForInput(const QList &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 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 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 diff --git a/src/input.cpp b/src/input.cpp index 289bcff776..c2f5426af9 100644 --- a/src/input.cpp +++ b/src/input.cpp @@ -1666,6 +1666,7 @@ public: workspace()->tabbox()->keyPress(event->modifiers() | event->key()); } else if (static_cast(event)->modifiersRelevantForGlobalShortcuts() == Qt::NoModifier) { workspace()->tabbox()->modifiersReleased(); + return false; } return true; } diff --git a/src/input.h b/src/input.h index 65dcbb7256..dfe2d0ffe3 100644 --- a/src/input.h +++ b/src/input.h @@ -381,6 +381,7 @@ enum Order { Popup, Decoration, WindowAction, + XWayland, InternalWindow, InputMethod, Forward, diff --git a/src/xwayland/xwayland.cpp b/src/xwayland/xwayland.cpp index d87ab9460f..49f0526f70 100644 --- a/src/xwayland/xwayland.cpp +++ b/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(); - input()->installInputEventSpy(m_inputSpy.get()); - m_inputSpy->setMode(options->xwaylandEavesdrops(), options->xwaylandEavesdropsMouse()); + m_inputFilter = std::make_unique(); + m_inputFilter->setMode(options->xwaylandEavesdrops(), options->xwaylandEavesdropsMouse()); + input()->installInputEventFilter(m_inputFilter.get()); } else { - input()->uninstallInputEventSpy(m_inputSpy.get()); - m_inputSpy.reset(); + m_inputFilter.reset(); } } diff --git a/src/xwayland/xwayland.h b/src/xwayland/xwayland.h index 6e8e758c29..3cec7fce27 100644 --- a/src/xwayland/xwayland.h +++ b/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 m_inputSpy; + std::unique_ptr m_inputFilter; Q_DISABLE_COPY(Xwayland) };