From 8deea58fd7554bec691eacc6cf9de7acbc51b198 Mon Sep 17 00:00:00 2001 From: Wolfgang Müller Date: Fri, 22 Nov 2024 14:09:17 +0100 Subject: desktop-plasma: Add patchset fixing tabbox behaviour See [1]. Merged upstream in [2] but not yet slated for release. [1] https://invent.kde.org/plasma/kwin/-/merge_requests/6697 [2] https://invent.kde.org/plasma/kwin/-/commit/db873bbd5b8c31de9142de66ed0da6db937bc662 --- ...1-tabbox-Correctly-reset-an-invalid-index.patch | 55 +++++++ ...-skip-active-window-if-it-is-in-the-clien.patch | 121 +++++++++++++++ ...dd-tabbox-test-for-when-the-client-is-out.patch | 165 +++++++++++++++++++++ 3 files changed, 341 insertions(+) create mode 100644 desktop-plasma/patches/kde-plasma/kwin/0001-tabbox-Correctly-reset-an-invalid-index.patch create mode 100644 desktop-plasma/patches/kde-plasma/kwin/0002-tabbox-Only-skip-active-window-if-it-is-in-the-clien.patch create mode 100644 desktop-plasma/patches/kde-plasma/kwin/0003-autotests-Add-tabbox-test-for-when-the-client-is-out.patch diff --git a/desktop-plasma/patches/kde-plasma/kwin/0001-tabbox-Correctly-reset-an-invalid-index.patch b/desktop-plasma/patches/kde-plasma/kwin/0001-tabbox-Correctly-reset-an-invalid-index.patch new file mode 100644 index 0000000..f188b33 --- /dev/null +++ b/desktop-plasma/patches/kde-plasma/kwin/0001-tabbox-Correctly-reset-an-invalid-index.patch @@ -0,0 +1,55 @@ +From ce4a56a522bf073512878392f7bc7e6a833ebfe3 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Wolfgang=20M=C3=BCller?= +Date: Wed, 23 Oct 2024 16:49:21 +0200 +Subject: [PATCH 1/3] tabbox: Correctly reset an invalid index + +When resetting the tabbox before making it visible, we want to set the +index to the active window as our starting point. It might happen that +the active window is not part of the selection model, for example when +the tabbox is showing only windows on other desktops or activities. If +this happens the index is invalid, so we want to take care to set the +index to the first item instead. + +Currently we handle this by first attempting to set the index to the +active window with setCurrentClient() and then afterwards checking +whether currentIndex() is valid. However, setCurrentIndex() only changes +the index if it is valid, so the index is never changed and remains at +its previous valid position. This makes the following check with +currentIndex() irrelevant and the index is never reset to the first +item. + +What this means in practice is that a tabbox whose client list does not +contain the active window initially has whichever client selected that +corresponds to the index set in a previous invocation of tabbox. + +To fix this, look up the index of the active window first and only set +the current index to it if it is valid. Otherwise set the current index +to the first item. +--- + src/tabbox/tabbox.cpp | 10 ++++------ + 1 file changed, 4 insertions(+), 6 deletions(-) + +diff --git a/src/tabbox/tabbox.cpp b/src/tabbox/tabbox.cpp +index 42d80f77d3..f3bbfa99bc 100644 +--- a/src/tabbox/tabbox.cpp ++++ b/src/tabbox/tabbox.cpp +@@ -424,12 +424,10 @@ void TabBox::reset(bool partial_reset) + { + m_tabBox->createModel(partial_reset); + if (!partial_reset) { +- if (Workspace::self()->activeWindow()) { +- setCurrentClient(Workspace::self()->activeWindow()); +- } +- // it's possible that the active client is not part of the model +- // in that case the index is invalid +- if (!m_tabBox->currentIndex().isValid()) { ++ const QModelIndex activeIndex = m_tabBox->index(workspace()->activeWindow()); ++ if (activeIndex.isValid()) { ++ setCurrentIndex(activeIndex); ++ } else { + setCurrentIndex(m_tabBox->first()); + } + } else { +-- +2.47.0 + diff --git a/desktop-plasma/patches/kde-plasma/kwin/0002-tabbox-Only-skip-active-window-if-it-is-in-the-clien.patch b/desktop-plasma/patches/kde-plasma/kwin/0002-tabbox-Only-skip-active-window-if-it-is-in-the-clien.patch new file mode 100644 index 0000000..1ed9912 --- /dev/null +++ b/desktop-plasma/patches/kde-plasma/kwin/0002-tabbox-Only-skip-active-window-if-it-is-in-the-clien.patch @@ -0,0 +1,121 @@ +From 52afb2a52b17b82fd85150f40200b186517e49f4 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Wolfgang=20M=C3=BCller?= +Date: Wed, 23 Oct 2024 18:45:46 +0200 +Subject: [PATCH 2/3] tabbox: Only skip active window if it is in the client + list + +When invoking the tabbox we initially advance the selection by one, +assuming the first item in the client list is the active window which +we'd like to skip. However, since we allow the user to apply a filter to +the clients displayed in the list, we might end up with the active +window missing from the current client list and skipping over an +unrelated client. When using focus chain switching this means we skip +over the last focused window, making quick switching between two windows +impossible. + +Instead of advancing the selection unconditionally, make sure not to +advance if the client list does not contain the active window and we are +in the initial invocation of the tabbox in navigatingThroughWindows(). +--- + src/tabbox/tabbox.cpp | 25 +++++++++++++++++++------ + src/tabbox/tabbox.h | 7 ++++++- + 2 files changed, 25 insertions(+), 7 deletions(-) + +diff --git a/src/tabbox/tabbox.cpp b/src/tabbox/tabbox.cpp +index f3bbfa99bc..5d6b2cb569 100644 +--- a/src/tabbox/tabbox.cpp ++++ b/src/tabbox/tabbox.cpp +@@ -478,6 +478,11 @@ void TabBox::setCurrentIndex(QModelIndex index, bool notifyEffects) + } + } + ++bool TabBox::haveActiveClient() ++{ ++ return m_tabBox->index(m_tabBox->activeClient()).isValid(); ++} ++ + void TabBox::show() + { + Q_EMIT tabBoxAdded(m_tabBoxMode); +@@ -808,9 +813,7 @@ void TabBox::navigatingThroughWindows(bool forward, const QKeySequence &shortcut + CDEWalkThroughWindows(forward); + } else { + if (areModKeysDepressed(shortcut)) { +- if (startKDEWalkThroughWindows(mode)) { +- KDEWalkThroughWindows(forward); +- } ++ startKDEWalkThroughWindows(forward, mode); + } else { + // if the shortcut has no modifiers, don't show the tabbox, + // don't grab, but simply go to the next window +@@ -898,7 +901,7 @@ bool TabBox::toggleMode(TabBoxMode mode) + return true; + } + +-bool TabBox::startKDEWalkThroughWindows(TabBoxMode mode) ++bool TabBox::startKDEWalkThroughWindows(bool forward, TabBoxMode mode) + { + if (!establishTabBoxGrab()) { + return false; +@@ -911,13 +914,19 @@ bool TabBox::startKDEWalkThroughWindows(TabBoxMode mode) + + setMode(mode); + reset(); ++ ++ if (haveActiveClient()) { ++ nextPrev(forward); ++ } ++ ++ delayedShow(); ++ + return true; + } + + void TabBox::KDEWalkThroughWindows(bool forward) + { + nextPrev(forward); +- delayedShow(); + } + + void TabBox::CDEWalkThroughWindows(bool forward) +@@ -979,7 +988,11 @@ void TabBox::KDEOneStepThroughWindows(bool forward, TabBoxMode mode) + { + setMode(mode); + reset(); +- nextPrev(forward); ++ ++ if (haveActiveClient()) { ++ nextPrev(forward); ++ } ++ + if (Window *c = currentClient()) { + Workspace::self()->activateWindow(c); + shadeActivate(c); +diff --git a/src/tabbox/tabbox.h b/src/tabbox/tabbox.h +index ec4ca2ba28..72e43b3a2e 100644 +--- a/src/tabbox/tabbox.h ++++ b/src/tabbox/tabbox.h +@@ -93,6 +93,11 @@ public: + */ + void setCurrentClient(Window *newClient); + ++ /** ++ * Return whether the active client is present in the client list. ++ */ ++ bool haveActiveClient(); ++ + void setMode(TabBoxMode mode); + TabBoxMode mode() const + { +@@ -228,7 +233,7 @@ private: + explicit TabBox(QObject *parent); + void loadConfig(const KConfigGroup &config, TabBoxConfig &tabBoxConfig); + +- bool startKDEWalkThroughWindows(TabBoxMode mode); // TabBoxWindowsMode | TabBoxWindowsAlternativeMode ++ bool startKDEWalkThroughWindows(bool forward, TabBoxMode mode); // TabBoxWindowsMode | TabBoxWindowsAlternativeMode + void navigatingThroughWindows(bool forward, const QKeySequence &shortcut, TabBoxMode mode); // TabBoxWindowsMode | TabBoxWindowsAlternativeMode + void KDEWalkThroughWindows(bool forward); + void CDEWalkThroughWindows(bool forward); +-- +2.47.0 + diff --git a/desktop-plasma/patches/kde-plasma/kwin/0003-autotests-Add-tabbox-test-for-when-the-client-is-out.patch b/desktop-plasma/patches/kde-plasma/kwin/0003-autotests-Add-tabbox-test-for-when-the-client-is-out.patch new file mode 100644 index 0000000..7ca29af --- /dev/null +++ b/desktop-plasma/patches/kde-plasma/kwin/0003-autotests-Add-tabbox-test-for-when-the-client-is-out.patch @@ -0,0 +1,165 @@ +From 31d2ac0ac4c1f6a94422dddadeb4f6199d3f42ca Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Wolfgang=20M=C3=BCller?= +Date: Thu, 24 Oct 2024 14:09:30 +0200 +Subject: [PATCH 3/3] autotests: Add tabbox test for when the client is outside + the model + +The last two commits fix tabbox behaviour when the active client is +missing from the client list (e.g. when the tabbox only shows windows on +other screens, desktops, or activities). Since both commits depend on +each other, only add the test now. +--- + autotests/integration/tabbox_test.cpp | 124 ++++++++++++++++++++++++++ + 1 file changed, 124 insertions(+) + +diff --git a/autotests/integration/tabbox_test.cpp b/autotests/integration/tabbox_test.cpp +index 63a2acc6ec..1ccda0cd28 100644 +--- a/autotests/integration/tabbox_test.cpp ++++ b/autotests/integration/tabbox_test.cpp +@@ -8,6 +8,7 @@ + */ + #include "kwin_wayland_test.h" + ++#include "core/outputbackend.h" + #include "input.h" + #include "pointer_input.h" + #include "tabbox/tabbox.h" +@@ -38,6 +39,7 @@ private Q_SLOTS: + void testMoveBackward(); + void testCapsLock(); + void testKeyboardFocus(); ++ void testActiveClientOutsideModel(); + }; + + void TabBoxTest::initTestCase() +@@ -289,5 +291,127 @@ void TabBoxTest::testKeyboardFocus() + QVERIFY(enteredSpy.wait()); + } + ++void TabBoxTest::testActiveClientOutsideModel() ++{ ++#if !KWIN_BUILD_GLOBALSHORTCUTS ++ QSKIP("Can't test shortcuts without shortcuts"); ++ return; ++#endif ++ ++ // This test verifies behaviour when the active client is outside the ++ // client list model: ++ // ++ // 1) reset() should correctly set the index to 0 if the active window is ++ // not part of the client list. ++ // 2) the selection should not be advanced initially if the active window ++ // is not part of the client list. ++ ++ const auto outputs = kwinApp()->outputBackend()->outputs(); ++ ++ // Initially, set up MultiScreenMode such that alt+tab will only switch ++ // within windows on the same screen. ++ KConfigGroup group = kwinApp()->config()->group(QStringLiteral("TabBox")); ++ group.writeEntry("MultiScreenMode", "1"); ++ group.sync(); ++ workspace()->slotReconfigure(); ++ ++ // Create a window on the left output ++ std::unique_ptr leftSurface1(Test::createSurface()); ++ std::unique_ptr leftShellSurface1(Test::createXdgToplevelSurface(leftSurface1.get())); ++ auto l1 = Test::renderAndWaitForShown(leftSurface1.get(), QSize(100, 50), Qt::blue); ++ l1->move(QPointF(50, 100)); ++ QVERIFY(l1); ++ QVERIFY(l1->isActive()); ++ QCOMPARE(l1->output(), outputs[0]); ++ ++ // Create three windows on the right output ++ std::unique_ptr rightSurface1(Test::createSurface()); ++ std::unique_ptr rightShellSurface1(Test::createXdgToplevelSurface(rightSurface1.get())); ++ auto r1 = Test::renderAndWaitForShown(rightSurface1.get(), QSize(100, 50), Qt::blue); ++ r1->move(QPointF(1280 + 50, 100)); ++ QVERIFY(r1); ++ QVERIFY(r1->isActive()); ++ QCOMPARE(r1->output(), outputs[1]); ++ std::unique_ptr rightSurface2(Test::createSurface()); ++ std::unique_ptr rightShellSurface2(Test::createXdgToplevelSurface(rightSurface2.get())); ++ auto r2 = Test::renderAndWaitForShown(rightSurface2.get(), QSize(100, 50), Qt::red); ++ r2->move(QPointF(1280 + 50, 100)); ++ QVERIFY(r2); ++ QVERIFY(r2->isActive()); ++ QCOMPARE(r2->output(), outputs[1]); ++ std::unique_ptr rightSurface3(Test::createSurface()); ++ std::unique_ptr rightShellSurface3(Test::createXdgToplevelSurface(rightSurface3.get())); ++ auto r3 = Test::renderAndWaitForShown(rightSurface3.get(), QSize(100, 50), Qt::red); ++ r3->move(QPointF(1280 + 50, 100)); ++ QVERIFY(r3); ++ QVERIFY(r3->isActive()); ++ QCOMPARE(r3->output(), outputs[1]); ++ ++ // Focus r3 such that we're on the right output ++ input()->pointer()->warp(r3->frameGeometry().center()); ++ QCOMPARE(workspace()->activeOutput(), outputs[1]); ++ ++ // Setup tabbox signal spies ++ QSignalSpy tabboxAddedSpy(workspace()->tabbox(), &TabBox::TabBox::tabBoxAdded); ++ QSignalSpy tabboxClosedSpy(workspace()->tabbox(), &TabBox::TabBox::tabBoxClosed); ++ ++ // Press Alt+Tab, this will only show clients on the same output ++ quint32 timestamp = 0; ++ Test::keyboardKeyPressed(KEY_LEFTALT, timestamp++); ++ QCOMPARE(input()->keyboardModifiers(), Qt::AltModifier); ++ Test::keyboardKeyPressed(KEY_TAB, timestamp++); ++ Test::keyboardKeyReleased(KEY_TAB, timestamp++); ++ ++ QVERIFY(tabboxAddedSpy.wait()); ++ QVERIFY(workspace()->tabbox()->isGrabbed()); ++ ++ // Release Alt+Tab. This will have moved our index to 1 and focused r2 (the ++ // previously created window) ++ Test::keyboardKeyReleased(KEY_LEFTALT, timestamp++); ++ QCOMPARE(tabboxClosedSpy.count(), 1); ++ QCOMPARE(workspace()->tabbox()->isGrabbed(), false); ++ QCOMPARE(workspace()->activeWindow(), r2); ++ ++ // Now reconfigure MultiScreenMode such that alt+tab will only switch ++ // between windows on the other screen ++ group.writeEntry("MultiScreenMode", 2); ++ group.sync(); ++ workspace()->slotReconfigure(); ++ ++ // Activate and focus l1 to switch to the left output ++ workspace()->activateWindow(l1); ++ QCOMPARE(workspace()->activeWindow(), l1); ++ input()->pointer()->warp(l1->frameGeometry().center()); ++ QCOMPARE(workspace()->activeOutput(), outputs[0]); ++ ++ // Press Alt+Tab, this will show only clients on the other output. Our old ++ // index from the last invocation of tabbox should be reset to 0 since the ++ // active window (l1) cannot be located in the current client list ++ Test::keyboardKeyPressed(KEY_LEFTALT, timestamp++); ++ QCOMPARE(input()->keyboardModifiers(), Qt::AltModifier); ++ Test::keyboardKeyPressed(KEY_TAB, timestamp++); ++ Test::keyboardKeyReleased(KEY_TAB, timestamp++); ++ ++ QVERIFY(tabboxAddedSpy.wait()); ++ QVERIFY(workspace()->tabbox()->isGrabbed()); ++ ++ // Release Alt. With a correctly reset index we should start from the ++ // beginning, skip advancing one window and focus r2 - the last window in ++ // focus on the other output ++ Test::keyboardKeyReleased(KEY_LEFTALT, timestamp++); ++ QCOMPARE(tabboxClosedSpy.count(), 2); ++ QCOMPARE(workspace()->tabbox()->isGrabbed(), false); ++ QCOMPARE(workspace()->activeWindow(), r2); ++ ++ rightSurface3.reset(); ++ QVERIFY(Test::waitForWindowClosed(r3)); ++ rightSurface2.reset(); ++ QVERIFY(Test::waitForWindowClosed(r2)); ++ rightSurface1.reset(); ++ QVERIFY(Test::waitForWindowClosed(r1)); ++ leftSurface1.reset(); ++ QVERIFY(Test::waitForWindowClosed(l1)); ++} ++ + WAYLANDTEST_MAIN(TabBoxTest) + #include "tabbox_test.moc" +-- +2.47.0 + -- cgit v1.2.3-2-gb3c3