From 540002579677359a5ee5394204b31b0fdd96767e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Fri, 2 Aug 2024 13:44:17 +0200 Subject: [PATCH] widgets: Persist window Qt::Popup state when reparenting parent widget As a result of c956eb8eddb1b3608d7e3d332fbe55df5ec41578 we are now reparenting QWindows managed by QWidgets when the widget itself or any of its parent widgets are reparented. When reparenting a child widget from being a child to a top level, or top level to child, we need to know the new top level state to determine if the QWindow should have a QWindow parent or not. As the window flags of the widget are in flux during the reparenting, we were using the new window flags of the reparented widget to determine this. However, for QWidget children of the widget that's being reparented we can't use the window flags of the reparented widget. We must use the flags of the child itself. These flags are not in flux, so we can use QWidget::windowFlags() directly. Failing to propagate the child widget window flags was causing us to lose the transient parent relationship to its parent QWindow for popup windows. Fixes: QTBUG-127641 Fixes: QTBUG-125149 Task-number: QTBUG-122747 Pick-to: 6.7 6.5 Change-Id: I6dbc5ff5ad019b0f9188516ff3d645e860a9627b Reviewed-by: Axel Spoerl Reviewed-by: Volodymyr Zibarov (cherry picked from commit 6c036012b5f2304a05af29f29daa7582603f79ae) Reviewed-by: Qt Cherry-pick Bot --- src/widgets/kernel/qwidget.cpp | 24 ++++++++++------- .../widgets/kernel/qwidget/tst_qwidget.cpp | 26 +++++++++++++++++++ 2 files changed, 40 insertions(+), 10 deletions(-) diff --git a/src/widgets/kernel/qwidget.cpp b/src/widgets/kernel/qwidget.cpp index 0cfd3ca93cc..00a2d5aa19a 100644 --- a/src/widgets/kernel/qwidget.cpp +++ b/src/widgets/kernel/qwidget.cpp @@ -10943,10 +10943,19 @@ void QWidgetPrivate::setParent_sys(QWidget *newparent, Qt::WindowFlags f) QWidget *parentWithWindow = closestParentWidgetWithWindowHandle(); // But if the widget is about to be destroyed we must skip the // widget itself, and only reparent children. - if (destroyWindow) + if (destroyWindow) { reparentWidgetWindowChildren(parentWithWindow); - else + } else { + // During reparentWidgetWindows() we need to know whether the reparented + // QWindow should be a top level (with a transient parent) or not. This + // widget has not updated its window flags yet, so we can't ask the widget + // directly at that point. Nor can we use the QWindow flags, as unlike QWidgets + // the QWindow flags always reflect Qt::Window, even for child windows. And + // we can't use QWindow::isTopLevel() either, as that depends on the parent, + // which we are in the process of updating. So we propagate the + // new flags of the reparented window here. reparentWidgetWindows(parentWithWindow, f); + } } bool explicitlyHidden = isExplicitlyHidden(); @@ -11003,13 +11012,6 @@ void QWidgetPrivate::reparentWidgetWindows(QWidget *parentWithWindow, Qt::Window if (QWindow *window = windowHandle()) { // Reparent this QWindow, and all QWindow children will follow if (parentWithWindow) { - // The reparented widget has not updated its window flags yet, - // so we can't ask the widget directly. And we can't use the - // QWindow flags, as unlike QWidgets the QWindow flags always - // reflect Qt::Window, even for child windows. And we can't use - // QWindow::isTopLevel() either, as that depends on the parent, - // which we are in the process of updating. So we propagate the - // new flags of the reparented window from setParent_sys(). if (windowFlags & Qt::Window) { // Top level windows can only have transient parents, // and the transient parent must be another top level. @@ -11040,7 +11042,9 @@ void QWidgetPrivate::reparentWidgetWindowChildren(QWidget *parentWithWindow) for (auto *child : std::as_const(children)) { if (auto *childWidget = qobject_cast(child)) { auto *childPrivate = QWidgetPrivate::get(childWidget); - childPrivate->reparentWidgetWindows(parentWithWindow); + // Child widgets with QWindows should always continue to be child + // windows, so we pass on the child's current window flags here. + childPrivate->reparentWidgetWindows(parentWithWindow, childWidget->windowFlags()); } } } diff --git a/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp b/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp index f22570bdaf0..dad4ef10d91 100644 --- a/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp +++ b/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp @@ -13857,6 +13857,7 @@ void tst_QWidget::reparentWindowHandles_data() QTest::addRow("top level to child") << 2; QTest::addRow("transient parent") << 3; QTest::addRow("window container") << 4; + QTest::addRow("popup") << 5; } void tst_QWidget::reparentWindowHandles() @@ -13992,6 +13993,31 @@ void tst_QWidget::reparentWindowHandles() child->setParent(&anotherTopLevel); QCOMPARE(window->parent(), anotherTopLevel.windowHandle()); + } + case 5: { + // Popup window that's a child of a widget that is + // reparented should keep being a (top level) popup. + + QWidget topLevel; + topLevel.setAttribute(Qt::WA_NativeWindow); + QVERIFY(topLevel.windowHandle()); + + QPointer child = new QWidget(&topLevel); + QVERIFY(!child->windowHandle()); + + QPointer leaf = new QWidget(child, Qt::Popup); + leaf->setAttribute(Qt::WA_DontCreateNativeAncestors); + leaf->setAttribute(Qt::WA_NativeWindow); + QVERIFY(leaf->windowHandle()); + + QWidget anotherTopLevel; + anotherTopLevel.setAttribute(Qt::WA_NativeWindow); + QVERIFY(anotherTopLevel.windowHandle()); + + child->setParent(&anotherTopLevel); + QCOMPARE(leaf->windowHandle()->parent(), nullptr); + QCOMPARE(leaf->windowHandle()->transientParent(), + anotherTopLevel.windowHandle()); } break; default: