From a910daf52678469b7e595da5fe54aa09ba2373c3 Mon Sep 17 00:00:00 2001 From: Axel Spoerl Date: Wed, 20 Mar 2024 18:10:45 +0100 Subject: [PATCH] QDockWidget::setFloating() - reparent floating tab to main window setFloating() called on the 2nd last dock widget in a floating tab didn't cause reparenting to the main window. The dock widget remained parented to a QDockWidgetGroupWindow, while no longer being part of floating tabs. The QDockWidgetGroupWindow would therefore have only one tab, which is an inconsistent state and causes crashes. Factor out the implementation of setFloating() to a new QDockWidgetPrivate::setFloating(). Reparent to the main window, if the dock widget is part of floating tabs. Add test functionality in tst_QDockWidget::setFloating(). Turn createFloatingTabs() into a macro, in order to QSKIP() in the test function's scope. Fixes: QTBUG-122394 Pick-to: 6.7 6.5 Change-Id: I3719785f310b131446cdea908f09b7195c992b3e Reviewed-by: Volker Hilsheimer --- src/widgets/widgets/qdockwidget.cpp | 56 ++++++++++++++++--- src/widgets/widgets/qdockwidget_p.h | 1 + .../widgets/qdockwidget/tst_qdockwidget.cpp | 51 ++++++++++++++--- 3 files changed, 92 insertions(+), 16 deletions(-) diff --git a/src/widgets/widgets/qdockwidget.cpp b/src/widgets/widgets/qdockwidget.cpp index 706306000c0..f3535255535 100644 --- a/src/widgets/widgets/qdockwidget.cpp +++ b/src/widgets/widgets/qdockwidget.cpp @@ -1448,22 +1448,60 @@ QDockWidget::DockWidgetFeatures QDockWidget::features() const void QDockWidget::setFloating(bool floating) { Q_D(QDockWidget); + d->setFloating(floating); +} +/*! + \internal implementation of setFloating + */ +void QDockWidgetPrivate::setFloating(bool floating) +{ + Q_Q(QDockWidget); // the initial click of a double-click may have started a drag... - if (d->state != nullptr) - d->endDrag(QDockWidgetPrivate::EndDragMode::Abort); + if (state != nullptr) + endDrag(QDockWidgetPrivate::EndDragMode::Abort); - QRect r = d->undockedGeometry; // Keep position when undocking for the first time. - if (floating && isVisible() && !r.isValid()) - r = QRect(mapToGlobal(QPoint(0, 0)), size()); + QRect r = undockedGeometry; + if (floating && q->isVisible() && !r.isValid()) + r = QRect(q->mapToGlobal(QPoint(0, 0)), q->size()); - d->setWindowState(floating, false, floating ? r : QRect()); + // Reparent, if setFloating() was called on a floating tab + // Reparenting has to happen before setWindowState. + // The reparented dock widget will inherit visibility from the floating tab. + // => Remember visibility and the necessity to update it. + enum class VisibilityRule { + NoUpdate, + Show, + Hide, + }; + + VisibilityRule updateRule = VisibilityRule::NoUpdate; + + if (floating && !q->isFloating()) { + if (auto *groupWindow = qobject_cast(q->parentWidget())) { + updateRule = groupWindow->isVisible() ? VisibilityRule::Show : VisibilityRule::Hide; + q->setParent(groupWindow->parentWidget()); + } + } + + setWindowState(floating, false, floating ? r : QRect()); if (floating && r.isNull()) { - if (x() < 0 || y() < 0) //may happen if we have been hidden - move(QPoint()); - setAttribute(Qt::WA_Moved, false); //we want it at the default position + if (q->x() < 0 || q->y() < 0) //may happen if we have been hidden + q->move(QPoint()); + q->setAttribute(Qt::WA_Moved, false); //we want it at the default position + } + + switch (updateRule) { + case VisibilityRule::NoUpdate: + break; + case VisibilityRule::Show: + q->show(); + break; + case VisibilityRule::Hide: + q->hide(); + break; } } diff --git a/src/widgets/widgets/qdockwidget_p.h b/src/widgets/widgets/qdockwidget_p.h index fa936599c69..6d3d3077298 100644 --- a/src/widgets/widgets/qdockwidget_p.h +++ b/src/widgets/widgets/qdockwidget_p.h @@ -104,6 +104,7 @@ public: void unplug(const QRect &rect); void plug(const QRect &rect); void setResizerActive(bool active); + void setFloating(bool floating); bool isAnimating() const; bool isTabbed() const; diff --git a/tests/auto/widgets/widgets/qdockwidget/tst_qdockwidget.cpp b/tests/auto/widgets/widgets/qdockwidget/tst_qdockwidget.cpp index af62d0d0899..88a7057d2ee 100644 --- a/tests/auto/widgets/widgets/qdockwidget/tst_qdockwidget.cpp +++ b/tests/auto/widgets/widgets/qdockwidget/tst_qdockwidget.cpp @@ -96,6 +96,20 @@ private: QPointer &d1, QPointer &d2, QList &path1, QList &path2) const; +#if defined(Q_OS_DARWIN) || defined(Q_OS_ANDROID) || defined(Q_OS_QNX) +#define qCreateFloatingTabs(mainWindow, centralWidget, d1, d2, path1, path2)\ + mainWindow = nullptr;\ + Q_UNUSED(path1);\ + Q_UNUSED(path2);\ + QSKIP("Platform not supported"); +#else +#define qCreateFloatingTabs(mainWindow, centralWidget, d1, d2, path1, path2)\ + createFloatingTabs(mainWindow, centralWidget, d1, d2, path1, path2);\ + std::unique_ptr up_mainWindow(mainWindow);\ + if (!platformSupportingRaise)\ + QSKIP("Platform not supporting raise(). Floating tab based tests will fail.") +#endif + static inline QPoint dragPoint(QDockWidget* dockWidget); static inline QPoint home1(QMainWindow* MainWindow) { return MainWindow->mapToGlobal(MainWindow->rect().topLeft() + QPoint(0.1 * MainWindow->width(), 0.1 * MainWindow->height())); } @@ -447,6 +461,23 @@ void tst_QDockWidget::setFloating() dw.setFloating(dw.isFloating()); QCOMPARE(spy.size(), 0); spy.clear(); + +#if defined(QT_BUILD_INTERNAL) && !defined(Q_OS_WIN) + // Check that setFloating() reparents the dock widget to the main window, + // in case it has a QDockWidgetGroupWindow parent + QPointer d1; + QPointer d2; + QPointer cent; + QMainWindow* mainWindow; + QList path1; + QList path2; + qCreateFloatingTabs(mainWindow, cent, d1, d2, path1, path2); + QVERIFY(qobject_cast(d1->parentWidget())); + QVERIFY(qobject_cast(d2->parentWidget())); + d1->setFloating(true); + QTRY_COMPARE(mainWindow, d1->parentWidget()); + QTRY_COMPARE(mainWindow, d2->parentWidget()); +#endif // defined(QT_BUILD_INTERNAL) && !defined(Q_OS_WIN) } void tst_QDockWidget::allowedAreas() @@ -1399,7 +1430,7 @@ void tst_QDockWidget::createFloatingTabs(QMainWindow* &mainWindow, QPointerwindowHandle()->handle()->raise(); if (!platformSupportingRaise) - QSKIP("Platform not supporting raise(). Floating tab based tests will fail."); + return; // remember paths to d1 and d2 QMainWindowLayout* layout = qobject_cast(mainWindow->layout()); @@ -1443,8 +1474,7 @@ void tst_QDockWidget::floatingTabs() QMainWindow* mainWindow; QList path1; QList path2; - createFloatingTabs(mainWindow, cent, d1, d2, path1, path2); - std::unique_ptr up_mainWindow(mainWindow); + qCreateFloatingTabs(mainWindow, cent, d1, d2, path1, path2); QCOMPARE(mainWindow->tabifiedDockWidgets(d1), {d2}); QCOMPARE(mainWindow->tabifiedDockWidgets(d2), {d1}); @@ -1497,9 +1527,13 @@ void tst_QDockWidget::floatingTabs() QTest::mouseClick(floatButton, Qt::LeftButton, Qt::KeyboardModifiers(), pos1); QTest::qWait(waitingTime); - // d1 must be floating again, while d2 is still in its GroupWindow + // d1 and d2 must be floating again QTRY_VERIFY(d1->isFloating()); - QTRY_VERIFY(!d2->isFloating()); + QTRY_VERIFY(d2->isFloating()); + + // d2 was the active tab, so d1 was not visible + QTRY_VERIFY(d1->isVisible()); + QTRY_VERIFY(d2->isVisible()); // Plug back into dock areas qCDebug(lcTestDockWidget) << "*** test plugging back to dock areas ***"; @@ -1562,8 +1596,7 @@ void tst_QDockWidget::deleteFloatingTabWithSingleDockWidget() QMainWindow* mainWindow; QList path1; QList path2; - createFloatingTabs(mainWindow, cent, d1, d2, path1, path2); - std::unique_ptr up_mainWindow(mainWindow); + qCreateFloatingTabs(mainWindow, cent, d1, d2, path1, path2); switch (removalReason) { case ChildRemovalReason::Destroyed: @@ -1698,6 +1731,9 @@ void tst_QDockWidget::closeAndDelete() if (QGuiApplication::platformName().startsWith(QLatin1String("wayland"), Qt::CaseInsensitive)) QSKIP("Test skipped on Wayland."); #ifdef QT_BUILD_INTERNAL + if (QSysInfo::productType() == "rhel") + QSKIP("Memory leak on RHEL 9.2 QTBUG-124559", TestFailMode::Abort); + // Create a mainwindow with a central widget and two dock widgets QPointer d1; QPointer d2; @@ -1992,6 +2028,7 @@ void tst_QDockWidget::saveAndRestore() QCOMPARE(d1->isFloating(), isFloating1); QCOMPARE(d2->isFloating(), isFloating2); +#undef qCreateFloatingTabs #endif // QT_BUILD_INTERNAL }