[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    [kwin] /: Add <number> to Wayland captions if the caption is the same
From:       Martin_Flöser <null () kde ! org>
Date:       2017-09-01 15:11:26
Message-ID: E1dnnbS-0007Tn-NU () code ! kde ! org
[Download RAW message or body]

Git commit 6685288d486925eeac5a2c14424812aa3c243902 by Martin Flöser.
Committed on 01/09/2017 at 15:02.
Pushed by graesslin into branch 'master'.

Add <number> 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 <number> 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

M  +8    -0    abstract_client.cpp
M  +23   -0    abstract_client.h
M  +48   -0    autotests/integration/shell_client_test.cpp
M  +2    -6    client.cpp
M  +6    -0    client.h
M  +18   -3    shell_client.cpp
M  +6    -0    shell_client.h

https://commits.kde.org/kwin/6685288d486925eeac5a2c14424812aa3c243902

diff --git a/abstract_client.cpp b/abstract_client.cpp
index 17e88ccdd..bd6262b42 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 a2fc9ad0d..d92c531ba 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<TabBox::TabBoxClientImpl> m_tabBoxClient;
diff --git a/autotests/integration/shell_client_test.cpp \
b/autotests/integration/shell_client_test.cpp index a0573537f..651e557ad 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> surface(Test::createSurface());
+    QScopedPointer<XdgShellSurface> \
shellSurface(qobject_cast<XdgShellSurface*>(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<Surface> surface2(Test::createSurface());
+    QScopedPointer<XdgShellSurface> \
shellSurface2(qobject_cast<XdgShellSurface*>(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<Surface> surface3(Test::createSurface());
+    QScopedPointer<XdgShellSurface> \
shellSurface3(qobject_cast<XdgShellSurface*>(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<Surface> surface4(Test::createSurface());
+    QScopedPointer<XdgShellSurface> \
shellSurface4(qobject_cast<XdgShellSurface*>(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<bool>("socketMode");
diff --git a/client.cpp b/client.cpp
index b785fb085..ad8d48aee 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 b77c8cdf8..0e90a1be4 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 6f11b225d..381a11b85 100644
--- a/shell_client.cpp
+++ b/shell_client.cpp
@@ -104,12 +104,19 @@ template <class T>
 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 401de11ae..a5ad7544e 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;


[prev in list] [next in list] [prev in thread] [next in thread]