From aac5d562fbcfcea7124a71bbdf9ea647e6114f2d Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Thu, 11 Jan 2024 16:52:26 +0200 Subject: [PATCH] Drop "" window caption suffix The current implementation of the `` suffix is still buggy and its benefits are doubtful. One could argue that visual aids such as window thumbnails or highlighting the windows are better. On its own, these numbers don't have strong connections to the windows and can change on a whim. --- autotests/integration/x11_window_test.cpp | 75 ------------------- autotests/integration/xdgshellwindow_test.cpp | 47 ------------ src/internalwindow.cpp | 22 +----- src/waylandwindow.cpp | 28 ++----- src/window.cpp | 8 -- src/window.h | 6 -- src/x11window.cpp | 14 +--- 7 files changed, 11 insertions(+), 189 deletions(-) diff --git a/autotests/integration/x11_window_test.cpp b/autotests/integration/x11_window_test.cpp index aa48b26d8d..19de48edd3 100644 --- a/autotests/integration/x11_window_test.cpp +++ b/autotests/integration/x11_window_test.cpp @@ -43,7 +43,6 @@ private Q_SLOTS: void testFocusInWithWaylandLastActiveWindow(); void testCaptionChanges(); void testCaptionWmName(); - void testCaptionMultipleWindows(); void testFullscreenWindowGroups(); void testActivateFocusedWindow(); void testReentrantMoveResize(); @@ -782,80 +781,6 @@ void X11WindowTest::testCaptionWmName() QVERIFY(glxgears.waitForFinished()); } -void X11WindowTest::testCaptionMultipleWindows() -{ - QFETCH_GLOBAL(qreal, scale); - kwinApp()->setXwaylandScale(scale); - - // BUG 384760 - // create first window - Test::XcbConnectionPtr c = Test::createX11Connection(); - QVERIFY(!xcb_connection_has_error(c.get())); - const QRect windowGeometry(0, 0, 100, 200); - xcb_window_t windowId = xcb_generate_id(c.get()); - xcb_create_window(c.get(), XCB_COPY_FROM_PARENT, windowId, rootWindow(), - windowGeometry.x(), - windowGeometry.y(), - windowGeometry.width(), - windowGeometry.height(), - 0, XCB_WINDOW_CLASS_INPUT_OUTPUT, XCB_COPY_FROM_PARENT, 0, nullptr); - xcb_size_hints_t hints; - memset(&hints, 0, sizeof(hints)); - xcb_icccm_size_hints_set_position(&hints, 1, windowGeometry.x(), windowGeometry.y()); - xcb_icccm_size_hints_set_size(&hints, 1, windowGeometry.width(), windowGeometry.height()); - xcb_icccm_set_wm_normal_hints(c.get(), windowId, &hints); - NETWinInfo info(c.get(), windowId, kwinApp()->x11RootWindow(), NET::Properties(), NET::Properties2()); - info.setName("foo"); - xcb_map_window(c.get(), windowId); - xcb_flush(c.get()); - - QSignalSpy windowCreatedSpy(workspace(), &Workspace::windowAdded); - QVERIFY(windowCreatedSpy.wait()); - X11Window *window = windowCreatedSpy.first().first().value(); - QVERIFY(window); - QCOMPARE(window->window(), windowId); - QCOMPARE(window->caption(), QStringLiteral("foo")); - - // create second window with same caption - xcb_window_t w2 = xcb_generate_id(c.get()); - xcb_create_window(c.get(), XCB_COPY_FROM_PARENT, w2, rootWindow(), - windowGeometry.x(), - windowGeometry.y(), - windowGeometry.width(), - windowGeometry.height(), - 0, XCB_WINDOW_CLASS_INPUT_OUTPUT, XCB_COPY_FROM_PARENT, 0, nullptr); - xcb_icccm_set_wm_normal_hints(c.get(), w2, &hints); - NETWinInfo info2(c.get(), w2, kwinApp()->x11RootWindow(), NET::Properties(), NET::Properties2()); - info2.setName("foo"); - info2.setIconName("foo"); - xcb_map_window(c.get(), w2); - xcb_flush(c.get()); - - windowCreatedSpy.clear(); - QVERIFY(windowCreatedSpy.wait()); - X11Window *window2 = windowCreatedSpy.first().first().value(); - QVERIFY(window2); - QCOMPARE(window2->window(), w2); - QCOMPARE(window2->caption(), QStringLiteral("foo <2>\u200E")); - NETWinInfo info3(kwinApp()->x11Connection(), w2, kwinApp()->x11RootWindow(), NET::WMVisibleName | NET::WMVisibleIconName, NET::Properties2()); - QCOMPARE(QByteArray(info3.visibleName()), QByteArrayLiteral("foo <2>\u200E")); - QCOMPARE(QByteArray(info3.visibleIconName()), QByteArrayLiteral("foo <2>\u200E")); - - QSignalSpy captionChangedSpy(window2, &X11Window::captionChanged); - - NETWinInfo info4(c.get(), w2, kwinApp()->x11RootWindow(), NET::Properties(), NET::Properties2()); - info4.setName("foobar"); - info4.setIconName("foobar"); - xcb_map_window(c.get(), w2); - xcb_flush(c.get()); - - QVERIFY(captionChangedSpy.wait()); - QCOMPARE(window2->caption(), QStringLiteral("foobar")); - NETWinInfo info5(kwinApp()->x11Connection(), w2, kwinApp()->x11RootWindow(), NET::WMVisibleName | NET::WMVisibleIconName, NET::Properties2()); - QCOMPARE(QByteArray(info5.visibleName()), QByteArray()); - QTRY_COMPARE(QByteArray(info5.visibleIconName()), QByteArray()); -} - void X11WindowTest::testFullscreenWindowGroups() { // this test creates an X11 window and puts it to full screen diff --git a/autotests/integration/xdgshellwindow_test.cpp b/autotests/integration/xdgshellwindow_test.cpp index 488cc264c2..6417ef429a 100644 --- a/autotests/integration/xdgshellwindow_test.cpp +++ b/autotests/integration/xdgshellwindow_test.cpp @@ -72,7 +72,6 @@ private Q_SLOTS: void testHidden(); void testDesktopFileName(); void testCaptionSimplified(); - void testCaptionMultipleWindows(); void testUnresponsiveWindow_data(); void testUnresponsiveWindow(); void testAppMenu(); @@ -708,52 +707,6 @@ void TestXdgShellWindow::testCaptionSimplified() QCOMPARE(window->caption(), origTitle.simplified()); } -void TestXdgShellWindow::testCaptionMultipleWindows() -{ - std::unique_ptr surface(Test::createSurface()); - std::unique_ptr shellSurface(Test::createXdgToplevelSurface(surface.get())); - shellSurface->set_title(QStringLiteral("foo")); - auto window = Test::renderAndWaitForShown(surface.get(), QSize(100, 50), Qt::blue); - QVERIFY(window); - QCOMPARE(window->caption(), QStringLiteral("foo")); - QCOMPARE(window->captionNormal(), QStringLiteral("foo")); - QCOMPARE(window->captionSuffix(), QString()); - - std::unique_ptr surface2(Test::createSurface()); - std::unique_ptr shellSurface2(Test::createXdgToplevelSurface(surface2.get())); - shellSurface2->set_title(QStringLiteral("foo")); - auto c2 = Test::renderAndWaitForShown(surface2.get(), QSize(100, 50), Qt::blue); - QVERIFY(c2); - QCOMPARE(c2->caption(), QStringLiteral("foo <2>")); - QCOMPARE(c2->captionNormal(), QStringLiteral("foo")); - QCOMPARE(c2->captionSuffix(), QStringLiteral(" <2>")); - - std::unique_ptr surface3(Test::createSurface()); - std::unique_ptr shellSurface3(Test::createXdgToplevelSurface(surface3.get())); - shellSurface3->set_title(QStringLiteral("foo")); - auto c3 = Test::renderAndWaitForShown(surface3.get(), QSize(100, 50), Qt::blue); - QVERIFY(c3); - QCOMPARE(c3->caption(), QStringLiteral("foo <3>")); - QCOMPARE(c3->captionNormal(), QStringLiteral("foo")); - QCOMPARE(c3->captionSuffix(), QStringLiteral(" <3>")); - - std::unique_ptr surface4(Test::createSurface()); - std::unique_ptr shellSurface4(Test::createXdgToplevelSurface(surface4.get())); - shellSurface4->set_title(QStringLiteral("bar")); - auto c4 = Test::renderAndWaitForShown(surface4.get(), QSize(100, 50), Qt::blue); - QVERIFY(c4); - QCOMPARE(c4->caption(), QStringLiteral("bar")); - QCOMPARE(c4->captionNormal(), QStringLiteral("bar")); - QCOMPARE(c4->captionSuffix(), QString()); - QSignalSpy captionChangedSpy(c4, &Window::captionChanged); - shellSurface4->set_title(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 TestXdgShellWindow::testUnresponsiveWindow_data() { QTest::addColumn("shellInterface"); // see env selection in qwaylandintegration.cpp diff --git a/src/internalwindow.cpp b/src/internalwindow.cpp index 20a7a4f27c..8159c4086b 100644 --- a/src/internalwindow.cpp +++ b/src/internalwindow.cpp @@ -411,17 +411,9 @@ void InternalWindow::doInteractiveResizeSync(const QRectF &rect) void InternalWindow::updateCaption() { - const QString oldSuffix = m_captionSuffix; - const auto shortcut = shortcutCaptionSuffix(); - m_captionSuffix = shortcut; - if ((!isSpecialWindow() || isToolbar()) && findWindowWithSameCaption()) { - int i = 2; - do { - m_captionSuffix = shortcut + QLatin1String(" <") + QString::number(i) + QLatin1Char('>'); - i++; - } while (findWindowWithSameCaption()); - } - if (m_captionSuffix != oldSuffix) { + const QString suffix = shortcutCaptionSuffix(); + if (m_captionSuffix != suffix) { + m_captionSuffix = suffix; Q_EMIT captionChanged(); } } @@ -472,14 +464,8 @@ void InternalWindow::setCaption(const QString &caption) } m_captionNormal = caption; - - const QString oldCaptionSuffix = m_captionSuffix; - updateCaption(); - Q_EMIT captionNormalChanged(); - if (m_captionSuffix == oldCaptionSuffix) { - Q_EMIT captionChanged(); - } + Q_EMIT captionChanged(); } void InternalWindow::markAsMapped() diff --git a/src/waylandwindow.cpp b/src/waylandwindow.cpp index d05016fb2b..8a673e0d55 100644 --- a/src/waylandwindow.cpp +++ b/src/waylandwindow.cpp @@ -187,34 +187,19 @@ void WaylandWindow::updateResourceName() void WaylandWindow::updateCaption() { - const QString oldSuffix = m_captionSuffix; - const auto shortcut = shortcutCaptionSuffix(); - m_captionSuffix = shortcut; - if ((!isSpecialWindow() || isToolbar()) && findWindowWithSameCaption()) { - int i = 2; - do { - m_captionSuffix = shortcut + QLatin1String(" <") + QString::number(i) + QLatin1Char('>'); - i++; - } while (findWindowWithSameCaption()); - } - if (m_captionSuffix != oldSuffix) { + const QString suffix = shortcutCaptionSuffix(); + if (m_captionSuffix != suffix) { + m_captionSuffix = suffix; Q_EMIT captionChanged(); } } void WaylandWindow::setCaption(const QString &caption) { - const QString oldNormal = m_captionNormal; - const QString oldSuffix = m_captionSuffix; - - m_captionNormal = caption.simplified(); - updateCaption(); - - if (m_captionNormal != oldNormal) { + const QString simplified = caption.simplified(); + if (m_captionNormal != simplified) { + m_captionNormal = simplified; Q_EMIT captionNormalChanged(); - } - if (m_captionSuffix == oldSuffix) { - // Don't emit caption change twice it already got emitted by the changing suffix. Q_EMIT captionChanged(); } } @@ -297,7 +282,6 @@ void WaylandWindow::markAsMapped() { if (Q_UNLIKELY(!ready_for_painting)) { setupCompositing(); - updateCaption(); setReadyForPainting(); } } diff --git a/src/window.cpp b/src/window.cpp index 17e48ff59a..6d167b7ad2 100644 --- a/src/window.cpp +++ b/src/window.cpp @@ -3080,14 +3080,6 @@ QString Window::shortcutCaptionSuffix() const return QLatin1String(" {") + shortcut().toString() + QLatin1Char('}'); } -Window *Window::findWindowWithSameCaption() const -{ - auto fetchNameInternalPredicate = [this](const Window *cl) { - return (!cl->isSpecialWindow() || cl->isToolbar()) && cl != this && cl->captionNormal() == captionNormal() && cl->captionSuffix() == captionSuffix(); - }; - return workspace()->findWindow(fetchNameInternalPredicate); -} - QString Window::caption() const { QString cap = captionNormal() + captionSuffix(); diff --git a/src/window.h b/src/window.h index 396f12d4ae..5209cd87c8 100644 --- a/src/window.h +++ b/src/window.h @@ -1708,12 +1708,6 @@ protected: QString shortcutCaptionSuffix() const; virtual void updateCaption() = 0; - /** - * Looks for another Window with same captionNormal and captionSuffix. - * If no such Window exists @c nullptr is returned. - */ - Window *findWindowWithSameCaption() const; - void startShadeHoverTimer(); void startShadeUnhoverTimer(); void shadeHover(); diff --git a/src/x11window.cpp b/src/x11window.cpp index 14b10e503f..d2c96b5cf7 100644 --- a/src/x11window.cpp +++ b/src/x11window.cpp @@ -644,8 +644,6 @@ bool X11Window::manage(xcb_window_t w, bool isMapped) setupWindowRules(); connect(this, &X11Window::windowClassChanged, this, &X11Window::evaluateWindowRules); - updateCaption(); // in case the window type has been changed by a window rule and has been dropped - if (Xcb::Extensions::self()->isShapeAvailable()) { xcb_shape_select_input(kwinApp()->x11Connection(), window(), true); } @@ -2340,7 +2338,6 @@ void X11Window::setCaption(const QString &_s, bool force) } cap_normal = s; - bool reset_name = force; bool was_suffix = (!cap_suffix.isEmpty()); cap_suffix.clear(); QString machine_suffix; @@ -2351,16 +2348,7 @@ void X11Window::setCaption(const QString &_s, bool force) } QString shortcut_suffix = shortcutCaptionSuffix(); cap_suffix = machine_suffix + shortcut_suffix; - if ((!isSpecialWindow() || isToolbar()) && findWindowWithSameCaption()) { - int i = 2; - do { - cap_suffix = machine_suffix + QLatin1String(" <") + QString::number(i) + QLatin1Char('>') + LRM; - i++; - } while (findWindowWithSameCaption()); - info->setVisibleName(caption().toUtf8().constData()); - reset_name = false; - } - if ((was_suffix && cap_suffix.isEmpty()) || reset_name) { + if ((was_suffix && cap_suffix.isEmpty()) || force) { // If it was new window, it may have old value still set, if the window is reused info->setVisibleName(""); info->setVisibleIconName("");