From dee8a3b3a741ab13a3680a21304c2dc514e3d3ba Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Thu, 24 Aug 2023 16:59:22 +0300 Subject: [PATCH] Clear decoration focus when window closes If the window closes, we need to reset the focused decoration object. BUG: 473244 --- .../integration/decoration_input_test.cpp | 82 ++++++++++++++----- src/pointer_input.cpp | 36 ++++---- src/pointer_input.h | 1 + 3 files changed, 84 insertions(+), 35 deletions(-) diff --git a/autotests/integration/decoration_input_test.cpp b/autotests/integration/decoration_input_test.cpp index 2eb4c5997b..d05e027e2c 100644 --- a/autotests/integration/decoration_input_test.cpp +++ b/autotests/integration/decoration_input_test.cpp @@ -53,8 +53,9 @@ private Q_SLOTS: void cleanup(); void testAxis_data(); void testAxis(); - void testDoubleClick_data(); - void testDoubleClick(); + void testDoubleClickOnAllDesktops_data(); + void testDoubleClickOnAllDesktops(); + void testDoubleClickClose(); void testDoubleTap_data(); void testDoubleTap(); void testHover(); @@ -72,7 +73,7 @@ private Q_SLOTS: void testTooltipDoesntEatKeyEvents(); private: - std::pair> showWindow(); + std::tuple, Test::XdgToplevel *> showWindow(); }; #define MOTION(target) Test::pointerMotion(target, timestamp++) @@ -81,14 +82,14 @@ private: #define RELEASE Test::pointerButtonReleased(BTN_LEFT, timestamp++) -std::pair> DecorationInputTest::showWindow() +std::tuple, Test::XdgToplevel *> DecorationInputTest::showWindow() { #define VERIFY(statement) \ if (!QTest::qVerify((statement), #statement, "", __FILE__, __LINE__)) \ - return {nullptr, nullptr}; + return {nullptr, nullptr, nullptr}; #define COMPARE(actual, expected) \ if (!QTest::qCompare(actual, expected, #actual, #expected, __FILE__, __LINE__)) \ - return {nullptr, nullptr}; + return {nullptr, nullptr, nullptr}; std::unique_ptr surface{Test::createSurface()}; VERIFY(surface.get()); @@ -114,7 +115,7 @@ std::pair> DecorationInputT #undef VERIFY #undef COMPARE - return {window, std::move(surface)}; + return {window, std::move(surface), shellSurface}; } void DecorationInputTest::initTestCase() @@ -174,7 +175,7 @@ void DecorationInputTest::testAxis() { static constexpr double oneTick = 15; - const auto [window, surface] = showWindow(); + const auto [window, surface, shellSurface] = showWindow(); QVERIFY(window); QVERIFY(window->isDecorated()); QVERIFY(!window->noBorder()); @@ -211,7 +212,7 @@ void DecorationInputTest::testAxis() QVERIFY(!window->keepAbove()); } -void DecorationInputTest::testDoubleClick_data() +void DecorationInputTest::testDoubleClickOnAllDesktops_data() { QTest::addColumn("decoPoint"); QTest::addColumn("expectedSection"); @@ -221,9 +222,14 @@ void DecorationInputTest::testDoubleClick_data() QTest::newRow("topRight") << QPoint(499, 0) << Qt::TopRightSection; } -void KWin::DecorationInputTest::testDoubleClick() +void KWin::DecorationInputTest::testDoubleClickOnAllDesktops() { - const auto [window, surface] = showWindow(); + KConfigGroup group = kwinApp()->config()->group("Windows"); + group.writeEntry("TitlebarDoubleClickCommand", QStringLiteral("OnAllDesktops")); + group.sync(); + workspace()->slotReconfigure(); + + const auto [window, surface, shellSurface] = showWindow(); QVERIFY(window); QVERIFY(window->isDecorated()); QVERIFY(!window->noBorder()); @@ -261,6 +267,37 @@ void KWin::DecorationInputTest::testDoubleClick() QVERIFY(window->isOnAllDesktops()); } +void DecorationInputTest::testDoubleClickClose() +{ + // this test verifies that no crash occurs when double click is configured to close action + KConfigGroup group = kwinApp()->config()->group("Windows"); + group.writeEntry("TitlebarDoubleClickCommand", QStringLiteral("Close")); + group.sync(); + workspace()->slotReconfigure(); + + auto [window, surface, shellSurface] = showWindow(); + QVERIFY(window); + QVERIFY(window->isDecorated()); + quint32 timestamp = 1; + MOTION(QPoint(window->frameGeometry().center().x(), window->frameMargins().top() / 2.0)); + + connect(shellSurface, &Test::XdgToplevel::closeRequested, this, [&surface = surface]() { + surface.reset(); + }); + + // double click + QSignalSpy closedSpy(window, &Window::closed); + window->ref(); + PRESS; + RELEASE; + PRESS; + QVERIFY(closedSpy.wait()); + RELEASE; + + QVERIFY(window->isDeleted()); + window->unref(); +} + void DecorationInputTest::testDoubleTap_data() { QTest::addColumn("decoPoint"); @@ -273,7 +310,12 @@ void DecorationInputTest::testDoubleTap_data() void KWin::DecorationInputTest::testDoubleTap() { - const auto [window, surface] = showWindow(); + KConfigGroup group = kwinApp()->config()->group("Windows"); + group.writeEntry("TitlebarDoubleClickCommand", QStringLiteral("OnAllDesktops")); + group.sync(); + workspace()->slotReconfigure(); + + const auto [window, surface, shellSurface] = showWindow(); QVERIFY(window); QVERIFY(window->isDecorated()); QVERIFY(!window->noBorder()); @@ -315,7 +357,7 @@ void KWin::DecorationInputTest::testDoubleTap() void DecorationInputTest::testHover() { - const auto [window, surface] = showWindow(); + const auto [window, surface, shellSurface] = showWindow(); QVERIFY(window); QVERIFY(window->isDecorated()); QVERIFY(!window->noBorder()); @@ -374,7 +416,7 @@ void DecorationInputTest::testPressToMove_data() void DecorationInputTest::testPressToMove() { - const auto [window, surface] = showWindow(); + const auto [window, surface, shellSurface] = showWindow(); QVERIFY(window); QVERIFY(window->isDecorated()); QVERIFY(!window->noBorder()); @@ -431,7 +473,7 @@ void DecorationInputTest::testTapToMove_data() void DecorationInputTest::testTapToMove() { - const auto [window, surface] = showWindow(); + const auto [window, surface, shellSurface] = showWindow(); QVERIFY(window); QVERIFY(window->isDecorated()); QVERIFY(!window->noBorder()); @@ -495,7 +537,7 @@ void DecorationInputTest::testResizeOutsideWindow() workspace()->slotReconfigure(); // now create window - const auto [window, surface] = showWindow(); + const auto [window, surface, shellSurface] = showWindow(); QVERIFY(window); QVERIFY(window->isDecorated()); QVERIFY(!window->noBorder()); @@ -589,7 +631,7 @@ void DecorationInputTest::testModifierClickUnrestrictedMove() QCOMPARE(options->commandAll3(), Options::MouseUnrestrictedMove); // create a window - const auto [window, surface] = showWindow(); + const auto [window, surface, shellSurface] = showWindow(); QVERIFY(window); QVERIFY(window->isDecorated()); QVERIFY(!window->noBorder()); @@ -651,7 +693,7 @@ void DecorationInputTest::testModifierScrollOpacity() group.sync(); workspace()->slotReconfigure(); - const auto [window, surface] = showWindow(); + const auto [window, surface, shellSurface] = showWindow(); QVERIFY(window); QVERIFY(window->isDecorated()); QVERIFY(!window->noBorder()); @@ -709,7 +751,7 @@ void DecorationInputTest::testTouchEvents() { // this test verifies that the decoration gets a hover leave event on touch release // see BUG 386231 - const auto [window, surface] = showWindow(); + const auto [window, surface, shellSurface] = showWindow(); QVERIFY(window); QVERIFY(window->isDecorated()); QVERIFY(!window->noBorder()); @@ -755,7 +797,7 @@ void DecorationInputTest::testTooltipDoesntEatKeyEvents() QVERIFY(keyboard); QSignalSpy enteredSpy(keyboard, &KWayland::Client::Keyboard::entered); - const auto [window, surface] = showWindow(); + const auto [window, surface, shellSurface] = showWindow(); QVERIFY(window); QVERIFY(window->isDecorated()); QVERIFY(!window->noBorder()); diff --git a/src/pointer_input.cpp b/src/pointer_input.cpp index e74568d5b3..169df2ca54 100644 --- a/src/pointer_input.cpp +++ b/src/pointer_input.cpp @@ -479,6 +479,9 @@ void PointerInputRedirection::cleanupDecoration(Decoration::DecoratedClientImpl disconnect(m_decorationDestroyedConnection); m_decorationDestroyedConnection = QMetaObject::Connection(); + disconnect(m_decorationClosedConnection); + m_decorationClosedConnection = QMetaObject::Connection(); + if (old) { // send leave event to old decoration QHoverEvent event(QEvent::HoverLeave, QPointF(), QPointF()); @@ -494,22 +497,25 @@ void PointerInputRedirection::cleanupDecoration(Decoration::DecoratedClientImpl QCoreApplication::instance()->sendEvent(now->decoration(), &event); now->window()->processDecorationMove(pos, m_pos); - m_decorationGeometryConnection = connect( - decoration()->window(), &Window::frameGeometryChanged, this, [this]() { - // ensure maximize button gets the leave event when maximizing/restore a window, see BUG 385140 - const auto oldDeco = decoration(); - update(); - if (oldDeco && oldDeco == decoration() && !decoration()->window()->isInteractiveMove() && !decoration()->window()->isInteractiveResize() && !areButtonsPressed()) { - // position of window did not change, we need to send HoverMotion manually - const QPointF p = m_pos - decoration()->window()->pos(); - QHoverEvent event(QEvent::HoverMove, p, p); - QCoreApplication::instance()->sendEvent(decoration()->decoration(), &event); - } - }, - Qt::QueuedConnection); + m_decorationGeometryConnection = connect(decoration()->window(), &Window::frameGeometryChanged, this, [this]() { + // ensure maximize button gets the leave event when maximizing/restore a window, see BUG 385140 + const auto oldDeco = decoration(); + update(); + if (oldDeco && oldDeco == decoration() && !decoration()->window()->isInteractiveMove() && !decoration()->window()->isInteractiveResize() && !areButtonsPressed()) { + // position of window did not change, we need to send HoverMotion manually + const QPointF p = m_pos - decoration()->window()->pos(); + QHoverEvent event(QEvent::HoverMove, p, p); + QCoreApplication::instance()->sendEvent(decoration()->decoration(), &event); + } + }); + + auto resetDecoration = [this]() { + setDecoration(nullptr); // explicitly reset decoration if focus updates are blocked + update(); + }; - // if our decoration gets destroyed whilst it has focus, we pass focus on to the same window - m_decorationDestroyedConnection = connect(now, &QObject::destroyed, this, &PointerInputRedirection::update, Qt::QueuedConnection); + m_decorationClosedConnection = connect(decoration()->window(), &Window::closed, this, resetDecoration); + m_decorationDestroyedConnection = connect(now, &QObject::destroyed, this, resetDecoration); } void PointerInputRedirection::focusUpdate(Window *focusOld, Window *focusNow) diff --git a/src/pointer_input.h b/src/pointer_input.h index 8d74d09fee..c7ec6d5382 100644 --- a/src/pointer_input.h +++ b/src/pointer_input.h @@ -175,6 +175,7 @@ private: QMetaObject::Connection m_lockedPointerAboutToBeUnboundConnection; QMetaObject::Connection m_decorationGeometryConnection; QMetaObject::Connection m_decorationDestroyedConnection; + QMetaObject::Connection m_decorationClosedConnection; bool m_confined = false; bool m_locked = false; bool m_enableConstraints = true;