From 1e9b961761a9316ce2d5638bcaa458422e88f2e4 Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Tue, 23 Jul 2024 11:30:20 +0300 Subject: [PATCH] tabbox: Reset keyboard focus when grabbing input When the tabbox grabs input, the key events won't be sent to the client. It's not good for a couple of reasons: first, it can fool the client into repeating a previously pressed key; and the server side and the client side can be out of sync after the task switcher is dismissed. In order to handle that properly, this change makes the tabbox reset the keyboard focus when it's shown or hidden. When the task switcher is dismissed, an enter event will be sent with the current state of the pressed keys. Ideally, the same needs to be done with the pointer focus but it's challenging to achieve with the current input pipeline design. An input grab abstraction is needed to handle pointer focus when the task switcher is active. --- autotests/integration/tabbox_test.cpp | 45 ++++++++++++++++++++++++++- src/input.cpp | 7 ----- src/keyboard_input.cpp | 39 ++++++++++++++++------- src/keyboard_input.h | 2 ++ src/tabbox/tabbox.cpp | 12 ++++++- 5 files changed, 85 insertions(+), 20 deletions(-) diff --git a/autotests/integration/tabbox_test.cpp b/autotests/integration/tabbox_test.cpp index e6d18a784e..ad981e1c4f 100644 --- a/autotests/integration/tabbox_test.cpp +++ b/autotests/integration/tabbox_test.cpp @@ -16,6 +16,8 @@ #include "workspace.h" #include +#include +#include #include #include @@ -35,6 +37,7 @@ private Q_SLOTS: void testMoveForward(); void testMoveBackward(); void testCapsLock(); + void testKeyboardFocus(); }; void TabBoxTest::initTestCase() @@ -59,7 +62,7 @@ void TabBoxTest::initTestCase() void TabBoxTest::init() { - QVERIFY(Test::setupWaylandConnection()); + QVERIFY(Test::setupWaylandConnection(Test::AdditionalWaylandInterface::Seat)); workspace()->setActiveOutput(QPoint(640, 512)); KWin::input()->pointer()->warp(QPoint(640, 512)); } @@ -248,5 +251,45 @@ void TabBoxTest::testMoveBackward() QVERIFY(Test::waitForWindowClosed(c1)); } +void TabBoxTest::testKeyboardFocus() +{ + // This test verifies that the keyboard focus will be withdrawn from the currently activated + // window when the task switcher is active and restored once the task switcher is dismissed. + + QVERIFY(Test::waitForWaylandKeyboard()); + + std::unique_ptr keyboard(Test::waylandSeat()->createKeyboard()); + QSignalSpy enteredSpy(keyboard.get(), &KWayland::Client::Keyboard::entered); + QSignalSpy leftSpy(keyboard.get(), &KWayland::Client::Keyboard::left); + + // add a window + std::unique_ptr surface(Test::createSurface()); + std::unique_ptr shellSurface(Test::createXdgToplevelSurface(surface.get())); + Test::renderAndWaitForShown(surface.get(), QSize(100, 50), Qt::blue); + + // the keyboard focus will be moved to the surface after it's mapped + QVERIFY(enteredSpy.wait()); + + QSignalSpy tabboxAddedSpy(workspace()->tabbox(), &TabBox::TabBox::tabBoxAdded); + QSignalSpy tabboxClosedSpy(workspace()->tabbox(), &TabBox::TabBox::tabBoxClosed); + + // press alt+tab + quint32 timestamp = 0; + Test::keyboardKeyPressed(KEY_LEFTALT, timestamp++); + Test::keyboardKeyPressed(KEY_TAB, timestamp++); + Test::keyboardKeyReleased(KEY_TAB, timestamp++); + QVERIFY(tabboxAddedSpy.wait()); + + // the surface should have no keyboard focus anymore because tabbox grabs input + QCOMPARE(leftSpy.count(), 1); + + // release alt + Test::keyboardKeyReleased(KEY_LEFTALT, timestamp++); + QCOMPARE(tabboxClosedSpy.count(), 1); + + // the surface should regain keyboard focus after the tabbox is dismissed + QVERIFY(enteredSpy.wait()); +} + WAYLANDTEST_MAIN(TabBoxTest) #include "tabbox_test.moc" diff --git a/src/input.cpp b/src/input.cpp index acba7d7d0d..db4ffa6423 100644 --- a/src/input.cpp +++ b/src/input.cpp @@ -1657,9 +1657,6 @@ public: if (!workspace()->tabbox() || !workspace()->tabbox()->isGrabbed()) { return false; } - auto seat = waylandServer()->seat(); - seat->setFocusedKeyboardSurface(nullptr); - input()->pointer()->setEnableConstraints(false); // pass the key event to the seat, so that it has a proper model of the currently hold keys // this is important for combinations like alt+shift to ensure that shift is not considered pressed passToWaylandServer(event); @@ -1668,10 +1665,6 @@ public: workspace()->tabbox()->keyPress(event->modifiers() | event->key()); } else if (static_cast(event)->modifiersRelevantForGlobalShortcuts() == Qt::NoModifier) { workspace()->tabbox()->modifiersReleased(); - // update keyboard facus if the tabbox no longer grabs keys - if (!workspace()->tabbox() || !workspace()->tabbox()->isGrabbed()) { - input()->keyboard()->update(); - } } return true; } diff --git a/src/keyboard_input.cpp b/src/keyboard_input.cpp index 55b4cdbb32..71973700f4 100644 --- a/src/keyboard_input.cpp +++ b/src/keyboard_input.cpp @@ -26,6 +26,9 @@ #if KWIN_BUILD_SCREENLOCKER #include #endif +#if KWIN_BUILD_TABBOX +#include "tabbox/tabbox.h" +#endif // Frameworks #include // Qt @@ -184,14 +187,8 @@ void KeyboardInputRedirection::reconfigure() } } -void KeyboardInputRedirection::update() +Window *KeyboardInputRedirection::pickFocus() const { - if (!m_inited) { - return; - } - auto seat = waylandServer()->seat(); - // TODO: this needs better integration - Window *found = nullptr; if (waylandServer()->isScreenLocked()) { const QList &stacking = Workspace::self()->stackingOrder(); if (!stacking.isEmpty()) { @@ -209,13 +206,33 @@ void KeyboardInputRedirection::update() if (!t->readyForPainting()) { continue; } - found = t; - break; + return t; } while (it != stacking.begin()); } - } else if (!input()->isSelectingWindow()) { - found = workspace()->activeWindow(); } + + if (input()->isSelectingWindow()) { + return nullptr; + } + +#if KWIN_BUILD_TABBOX + if (workspace()->tabbox()->isGrabbed()) { + return nullptr; + } +#endif + + return workspace()->activeWindow(); +} + +void KeyboardInputRedirection::update() +{ + if (!m_inited) { + return; + } + auto seat = waylandServer()->seat(); + + // TODO: this needs better integration + Window *found = pickFocus(); if (found && found->surface()) { if (found->surface() != seat->focusedKeyboardSurface()) { seat->setFocusedKeyboardSurface(found->surface()); diff --git a/src/keyboard_input.h b/src/keyboard_input.h index 46d4f8e6b7..bcaec68efe 100644 --- a/src/keyboard_input.h +++ b/src/keyboard_input.h @@ -63,6 +63,8 @@ Q_SIGNALS: void ledsChanged(KWin::LEDs); private: + Window *pickFocus() const; + InputRedirection *m_input; bool m_inited = false; const std::unique_ptr m_xkb; diff --git a/src/tabbox/tabbox.cpp b/src/tabbox/tabbox.cpp index 8e5cc30e53..e4ff5f2c22 100644 --- a/src/tabbox/tabbox.cpp +++ b/src/tabbox/tabbox.cpp @@ -884,6 +884,10 @@ bool TabBox::toggleMode(TabBoxMode mode) return false; } m_noModifierGrab = m_tabGrab = true; + + input()->keyboard()->update(); + input()->pointer()->setEnableConstraints(false); + setMode(mode); reset(); show(); @@ -897,6 +901,10 @@ bool TabBox::startKDEWalkThroughWindows(TabBoxMode mode) } m_tabGrab = true; m_noModifierGrab = false; + + input()->keyboard()->update(); + input()->pointer()->setEnableConstraints(false); + setMode(mode); reset(); return true; @@ -1092,9 +1100,11 @@ void TabBox::close(bool abort) removeTabBoxGrab(); } hide(abort); - input()->pointer()->setEnableConstraints(true); m_tabGrab = false; m_noModifierGrab = false; + + input()->keyboard()->update(); + input()->pointer()->setEnableConstraints(true); } void TabBox::accept(bool closeTabBox)