From 6685288d486925eeac5a2c14424812aa3c243902 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Fl=C3=B6ser?= Date: Sun, 20 Aug 2017 08:56:13 +0200 Subject: [PATCH] Add to Wayland captions if the caption is the same Summary: Bringing another caption feature from X11 to Wayland. If we have multiple windows with the same caption, starting from the second window a suffix is added. E.g. if we have three windows with caption "foo", the naming is: * foo * foo <2> * foo <3> The change tries to use as much shared code between the X11 and Wayland implementation. Unfortunately it's not possible to share completely as the X11 implementation does X11 specific things like editing the visible name. By sharing the code the numbering also works cross windowing system. That is if a window is called "foo" on X11, a new window on Wayland with caption "foo" will get adjusted to "foo <2>" and vice versa. The change also eliminates a duplicated signal for captionChanged in ShellClient (found by test case). By using the shared implementation on X11 side a bug gets fixed which got introduced with the support of "unresponsive", this is no longer considered and the numbering still works even if there is a window which is unresponsive. Test Plan: New test case and manual testing Reviewers: #kwin, #plasma Subscribers: plasma-devel, kwin Tags: #kwin Differential Revision: https://phabricator.kde.org/D7425 --- abstract_client.cpp | 8 ++++ abstract_client.h | 23 ++++++++++ autotests/integration/shell_client_test.cpp | 48 +++++++++++++++++++++ client.cpp | 8 +--- client.h | 6 +++ shell_client.cpp | 21 +++++++-- shell_client.h | 6 +++ 7 files changed, 111 insertions(+), 9 deletions(-) diff --git a/abstract_client.cpp b/abstract_client.cpp index 17e88ccdd9..bd6262b42f 100644 --- a/abstract_client.cpp +++ b/abstract_client.cpp @@ -1758,4 +1758,12 @@ QString AbstractClient::shortcutCaptionSuffix() const return QLatin1String(" {") + shortcut().toString() + QLatin1Char('}'); } +AbstractClient *AbstractClient::findClientWithSameCaption() const +{ + auto fetchNameInternalPredicate = [this](const AbstractClient *cl) { + return (!cl->isSpecialWindow() || cl->isToolbar()) && cl != this && cl->captionNormal() == captionNormal() && cl->captionSuffix() == captionSuffix(); + }; + return workspace()->findAbstractClient(fetchNameInternalPredicate); +} + } diff --git a/abstract_client.h b/abstract_client.h index a2fc9ad0d9..d92c531bac 100644 --- a/abstract_client.h +++ b/abstract_client.h @@ -343,7 +343,24 @@ public: } virtual void updateMouseGrab(); + /** + * @returns The caption consisting of @link{captionNormal} and @link{captionSuffix} + * @see captionNormal + * @see captionSuffix + **/ virtual QString caption(bool full = true) const = 0; + /** + * @returns The caption as set by the AbstractClient without any suffix. + * @see caption + * @see captionSuffix + **/ + virtual QString captionNormal() const = 0; + /** + * @returns The suffix added to the caption (e.g. shortcut, machine name, etc.) + * @see caption + * @see captionNormal + **/ + virtual QString captionSuffix() const = 0; virtual bool isCloseable() const = 0; // TODO: remove boolean trap virtual bool isShown(bool shaded_is_shown) const = 0; @@ -986,6 +1003,12 @@ protected: QString shortcutCaptionSuffix() const; virtual void updateCaption() = 0; + /** + * Looks for another AbstractClient with same @link{captionNormal} and @link{captionSuffix}. + * If no such AbstractClient exists @c nullptr is returned. + **/ + AbstractClient *findClientWithSameCaption() const; + private: void handlePaletteChange(); QSharedPointer m_tabBoxClient; diff --git a/autotests/integration/shell_client_test.cpp b/autotests/integration/shell_client_test.cpp index a0573537f1..651e557ad1 100644 --- a/autotests/integration/shell_client_test.cpp +++ b/autotests/integration/shell_client_test.cpp @@ -74,6 +74,7 @@ private Q_SLOTS: void testHidden(); void testDesktopFileName(); void testCaptionSimplified(); + void testCaptionMultipleWindows(); void testKillWindow_data(); void testKillWindow(); void testX11WindowId_data(); @@ -693,6 +694,53 @@ void TestShellClient::testCaptionSimplified() QCOMPARE(c->caption(), origTitle.simplified()); } +void TestShellClient::testCaptionMultipleWindows() +{ + QScopedPointer surface(Test::createSurface()); + QScopedPointer shellSurface(qobject_cast(Test::createShellSurface(Test::ShellSurfaceType::XdgShellV5, surface.data()))); + shellSurface->setTitle(QStringLiteral("foo")); + auto c = Test::renderAndWaitForShown(surface.data(), QSize(100, 50), Qt::blue); + QVERIFY(c); + QCOMPARE(c->caption(), QStringLiteral("foo")); + QCOMPARE(c->captionNormal(), QStringLiteral("foo")); + QCOMPARE(c->captionSuffix(), QString()); + + QScopedPointer surface2(Test::createSurface()); + QScopedPointer shellSurface2(qobject_cast(Test::createShellSurface(Test::ShellSurfaceType::XdgShellV5, surface2.data()))); + shellSurface2->setTitle(QStringLiteral("foo")); + auto c2 = Test::renderAndWaitForShown(surface2.data(), QSize(100, 50), Qt::blue); + QVERIFY(c2); + QCOMPARE(c2->caption(), QStringLiteral("foo <2>")); + QCOMPARE(c2->captionNormal(), QStringLiteral("foo")); + QCOMPARE(c2->captionSuffix(), QStringLiteral(" <2>")); + + QScopedPointer surface3(Test::createSurface()); + QScopedPointer shellSurface3(qobject_cast(Test::createShellSurface(Test::ShellSurfaceType::XdgShellV5, surface3.data()))); + shellSurface3->setTitle(QStringLiteral("foo")); + auto c3 = Test::renderAndWaitForShown(surface3.data(), QSize(100, 50), Qt::blue); + QVERIFY(c3); + QCOMPARE(c3->caption(), QStringLiteral("foo <3>")); + QCOMPARE(c3->captionNormal(), QStringLiteral("foo")); + QCOMPARE(c3->captionSuffix(), QStringLiteral(" <3>")); + + QScopedPointer surface4(Test::createSurface()); + QScopedPointer shellSurface4(qobject_cast(Test::createShellSurface(Test::ShellSurfaceType::XdgShellV5, surface4.data()))); + shellSurface4->setTitle(QStringLiteral("bar")); + auto c4 = Test::renderAndWaitForShown(surface4.data(), QSize(100, 50), Qt::blue); + QVERIFY(c4); + QCOMPARE(c4->caption(), QStringLiteral("bar")); + QCOMPARE(c4->captionNormal(), QStringLiteral("bar")); + QCOMPARE(c4->captionSuffix(), QString()); + QSignalSpy captionChangedSpy(c4, &ShellClient::captionChanged); + QVERIFY(captionChangedSpy.isValid()); + shellSurface4->setTitle(QStringLiteral("foo")); + QVERIFY(captionChangedSpy.wait()); + QCOMPARE(captionChangedSpy.count(), 1); + QCOMPARE(c4->caption(), QStringLiteral("foo <4>")); + QCOMPARE(c4->captionNormal(), QStringLiteral("foo")); + QCOMPARE(c4->captionSuffix(), QStringLiteral(" <4>")); +} + void TestShellClient::testKillWindow_data() { QTest::addColumn("socketMode"); diff --git a/client.cpp b/client.cpp index b785fb0852..ad8d48aeef 100644 --- a/client.cpp +++ b/client.cpp @@ -1463,16 +1463,12 @@ void Client::setCaption(const QString& _s, bool force) } QString shortcut_suffix = shortcutCaptionSuffix(); cap_suffix = machine_suffix + shortcut_suffix; - auto fetchNameInternalPredicate = [this](const Client *cl) { - return (!cl->isSpecialWindow() || cl->isToolbar()) && - cl != this && cl->caption() == caption(); - }; - if ((!isSpecialWindow() || isToolbar()) && workspace()->findClient(fetchNameInternalPredicate)) { + if ((!isSpecialWindow() || isToolbar()) && findClientWithSameCaption()) { int i = 2; do { cap_suffix = machine_suffix + QLatin1String(" <") + QString::number(i) + QLatin1Char('>') + LRM; i++; - } while (workspace()->findClient(fetchNameInternalPredicate)); + } while (findClientWithSameCaption()); info->setVisibleName(caption().toUtf8().constData()); reset_name = false; } diff --git a/client.h b/client.h index b77c8cdf86..0e90a1be43 100644 --- a/client.h +++ b/client.h @@ -220,6 +220,12 @@ public: inline bool isBlockingCompositing() { return blocks_compositing; } QString caption(bool full = true) const override; + QString captionNormal() const override { + return cap_normal; + } + QString captionSuffix() const override { + return cap_suffix; + } using AbstractClient::keyPressEvent; void keyPressEvent(uint key_code, xcb_timestamp_t time); // FRAME ?? diff --git a/shell_client.cpp b/shell_client.cpp index 6f11b225d4..381a11b85a 100644 --- a/shell_client.cpp +++ b/shell_client.cpp @@ -104,12 +104,19 @@ template void ShellClient::initSurface(T *shellSurface) { m_caption = shellSurface->title().simplified(); - connect(shellSurface, &T::titleChanged, this, &ShellClient::captionChanged); + // delay till end of init + QTimer::singleShot(0, this, &ShellClient::updateCaption); connect(shellSurface, &T::destroyed, this, &ShellClient::destroyClient); connect(shellSurface, &T::titleChanged, this, [this] (const QString &s) { + const auto oldSuffix = m_captionSuffix; m_caption = s.simplified(); - emit captionChanged(); + updateCaption(); + if (m_captionSuffix == oldSuffix) { + // don't emit caption change twice + // it already got emitted by the changing suffix + emit captionChanged(); + } } ); connect(shellSurface, &T::moveRequested, this, @@ -571,7 +578,15 @@ QString ShellClient::caption(bool full) const void ShellClient::updateCaption() { const QString oldSuffix = m_captionSuffix; - m_captionSuffix = shortcutCaptionSuffix(); + const auto shortcut = shortcutCaptionSuffix(); + m_captionSuffix = shortcut; + if ((!isSpecialWindow() || isToolbar()) && findClientWithSameCaption()) { + int i = 2; + do { + m_captionSuffix = shortcut + QLatin1String(" <") + QString::number(i) + QLatin1Char('>'); + i++; + } while (findClientWithSameCaption()); + } if (m_captionSuffix != oldSuffix) { emit captionChanged(); } diff --git a/shell_client.h b/shell_client.h index 401de11ae0..a5ad7544e5 100644 --- a/shell_client.h +++ b/shell_client.h @@ -64,6 +64,12 @@ public: void blockActivityUpdates(bool b = true) override; QString caption(bool full = true) const override; + QString captionNormal() const override { + return m_caption; + } + QString captionSuffix() const override { + return m_captionSuffix; + } void closeWindow() override; AbstractClient *findModal(bool allow_itself = false) override; bool isCloseable() const override;