Dialogs: Defer automatic window resolving until dialog is opened

The qml tool, as well as the QQuickView class are creating their own
window outside of the root qml file that is passed to them.

This caused an issue where no dialog were able to find a window when
componentComplete() was first called, which would make them return
early in the QPlatformDialogHelper::show() re-implementations, and
thus never show the dialogs when open() was called.

This patch solves the issue by waiting until open() is called, before
trying to find a window in any of the parent items.

Since Window always has a contentItem, it was also possible to simplify
the logic that searches for a window, by only trying to cast the parent
to QQuickItem*.

Another issue that came up, was the fact that dialogs with visible set
to true, aka:

FileDialog {
    visible: true
}

Should open as soon as possible upon its creation.
To get around this, open() is called during componentComplete() if a
window can be found during this time. Otherwise, the call to open() is
deferred until the nearest parent item emits the windowChanged() signal.

Note that calling setParentWindow() will now take priority over the
implicit window search, unless the argument for setParentWindow is
null. Meaning that moving a dialog to a different window, by modifying
parents, will not happen for dialogs that are explicitly binding a value
to parentWindow.

Fixes: QTBUG-106598
Pick-to: 6.5
Change-Id: Idd7c0ecdeea6cbf26e8d41168788563ee9794118
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>
(cherry picked from commit f233a5ff9d04daf3ba792cc196da7e5f190b415c)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit ba596daec67fb41d87d6c4a0b31f215fc0a1359d)

6 files changed, 229 insertions, 20 deletions

diff --git a/src/quickdialogs/quickdialogs/qquickabstractdialog.cpp b/src/quickdialogs/quickdialogs/qquickabstractdialog.cpp
index 857a75c7bd..088e7ae8d5 100644
--- a/src/quickdialogs/quickdialogs/qquickabstractdialog.cpp
+++ b/src/quickdialogs/quickdialogs/qquickabstractdialog.cpp
@@ -150,6 +150,8 @@ QWindow *QQuickAbstractDialog::parentWindow() const
 void QQuickAbstractDialog::setParentWindow(QWindow *window)
 {
     qCDebug(lcDialogs) << "set parent window to" << window;
+    m_parentWindowExplicitlySet = bool(window);
+
     if (m_parentWindow == window)
         return;
 
@@ -157,6 +159,17 @@ void QQuickAbstractDialog::setParentWindow(QWindow *window)
     emit parentWindowChanged();
 }
 
+void QQuickAbstractDialog::resetParentWindow()
+{
+    m_parentWindowExplicitlySet = false;
+
+    if (!m_parentWindow)
+        return;
+
+    m_parentWindow = nullptr;
+    emit parentWindowChanged();
+}
+
 /*!
     \qmlproperty string QtQuick.Dialogs::Dialog::title
 
@@ -287,7 +300,8 @@ void QQuickAbstractDialog::open()
         return;
 
     onShow(m_handle.get());
-    m_visible = m_handle->show(m_flags, m_modality, m_parentWindow);
+
+    m_visible = m_handle->show(m_flags, m_modality, windowForOpen());
     if (m_visible) {
         m_result = Rejected; // in case an accepted dialog gets re-opened, then closed
         emit visibleChanged();
@@ -310,6 +324,8 @@ void QQuickAbstractDialog::close()
     onHide(m_handle.get());
     m_handle->hide();
     m_visible = false;
+    if (!m_parentWindowExplicitlySet)
+        m_parentWindow = nullptr;
     emit visibleChanged();
 
     if (m_result == Accepted)
@@ -364,16 +380,22 @@ void QQuickAbstractDialog::componentComplete()
     qCDebug(lcDialogs) << "componentComplete";
     m_complete = true;
 
-    if (!m_parentWindow) {
-        qCDebug(lcDialogs) << "- no parent window; searching for one";
-        setParentWindow(findParentWindow());
-    }
+    if (!m_visibleRequested)
+        return;
 
-    if (m_visibleRequested) {
-        qCDebug(lcDialogs) << "visible was bound to true before component completion; opening dialog";
+    m_visibleRequested = false;
+
+    if (windowForOpen()) {
         open();
-        m_visibleRequested = false;
+        return;
     }
+
+    // Since visible were set to true by the user, we want the dialog to be open by default.
+    // There is no guarantee that the dialog will work when it exists in a object tree that lacks a window,
+    // and since qml components are sometimes instantiated before they're given a window
+    // (which is the case when using QQuickView), we want to delay the call to open(), until the window is provided.
+    if (const auto parentItem = findParentItem())
+        connect(parentItem, &QQuickItem::windowChanged, this, &QQuickAbstractDialog::deferredOpen, Qt::SingleShotConnection);
 }
 
 static const char *qmlTypeName(const QObject *object)
@@ -461,21 +483,33 @@ void QQuickAbstractDialog::onHide(QPlatformDialogHelper *dialog)
     Q_UNUSED(dialog);
 }
 
-QWindow *QQuickAbstractDialog::findParentWindow() const
+QQuickItem *QQuickAbstractDialog::findParentItem() const
 {
     QObject *obj = parent();
     while (obj) {
-        QWindow *window = qobject_cast<QWindow *>(obj);
-        if (window)
-            return window;
         QQuickItem *item = qobject_cast<QQuickItem *>(obj);
-        if (item && item->window())
-            return item->window();
+        if (item)
+            return item;
         obj = obj->parent();
     }
     return nullptr;
 }
 
+QWindow *QQuickAbstractDialog::windowForOpen() const
+{
+    if (m_parentWindowExplicitlySet)
+        return m_parentWindow;
+    else if (auto parentItem = findParentItem())
+        return parentItem->window();
+    return m_parentWindow;
+}
+
+void QQuickAbstractDialog::deferredOpen(QWindow *window)
+{
+    m_parentWindow = window;
+    open();
+}
+
 QT_END_NAMESPACE
 
 #include "moc_qquickabstractdialog_p.cpp"
diff --git a/src/quickdialogs/quickdialogs/qquickabstractdialog_p.h b/src/quickdialogs/quickdialogs/qquickabstractdialog_p.h
index f1d046eb89..30f164b12a 100644
--- a/src/quickdialogs/quickdialogs/qquickabstractdialog_p.h
+++ b/src/quickdialogs/quickdialogs/qquickabstractdialog_p.h
@@ -37,7 +37,7 @@ class Q_QUICKDIALOGS2_PRIVATE_EXPORT QQuickAbstractDialog : public QObject, publ
     Q_OBJECT
     Q_INTERFACES(QQmlParserStatus)
     Q_PROPERTY(QQmlListProperty<QObject> data READ data FINAL)
-    Q_PROPERTY(QWindow *parentWindow READ parentWindow WRITE setParentWindow NOTIFY parentWindowChanged FINAL)
+    Q_PROPERTY(QWindow *parentWindow READ parentWindow WRITE setParentWindow NOTIFY parentWindowChanged RESET resetParentWindow FINAL)
     Q_PROPERTY(QString title READ title WRITE setTitle NOTIFY titleChanged FINAL)
     Q_PROPERTY(Qt::WindowFlags flags READ flags WRITE setFlags NOTIFY flagsChanged FINAL)
     Q_PROPERTY(Qt::WindowModality modality READ modality WRITE setModality NOTIFY modalityChanged FINAL)
@@ -58,6 +58,7 @@ public:
 
     QWindow *parentWindow() const;
     void setParentWindow(QWindow *window);
+    void resetParentWindow();
 
     QString title() const;
     void setTitle(const QString &title);
@@ -106,12 +107,10 @@ protected:
     virtual void onShow(QPlatformDialogHelper *dialog);
     virtual void onHide(QPlatformDialogHelper *dialog);
 
-    QWindow *findParentWindow() const;
+    QQuickItem *findParentItem() const;
+    QWindow *windowForOpen() const;
+    void deferredOpen(QWindow *window);
 
-    bool m_visibleRequested = false;
-    bool m_visible = false;
-    bool m_complete = false;
-    bool m_firstShow = true;
     StandardCode m_result = Rejected;
     QWindow *m_parentWindow = nullptr;
     QString m_title;
@@ -120,6 +119,11 @@ protected:
     QQuickDialogType m_type = QQuickDialogType::FileDialog;
     QList<QObject *> m_data;
     std::unique_ptr<QPlatformDialogHelper> m_handle;
+    bool m_visibleRequested = false;
+    bool m_visible = false;
+    bool m_complete = false;
+    bool m_parentWindowExplicitlySet = false;
+    bool m_firstShow = true;
 };
 
 QT_END_NAMESPACE
diff --git a/tests/auto/quickdialogs/qquickcolordialogimpl/data/colorDialogWithoutWindow.qml b/tests/auto/quickdialogs/qquickcolordialogimpl/data/colorDialogWithoutWindow.qml
new file mode 100644
index 0000000000..5b9bab708c
--- /dev/null
+++ b/tests/auto/quickdialogs/qquickcolordialogimpl/data/colorDialogWithoutWindow.qml
@@ -0,0 +1,27 @@
+// Copyright (C) 2022 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR BSD-3-Clause
+
+import QtQuick
+import QtQuick.Controls
+import QtQuick.Dialogs
+
+Rectangle {
+    width: 480
+    height: 640
+    color: dialog.selectedColor
+
+    property alias dialog: dialog
+
+    function doneAccepted() {
+        dialog.done(ColorDialog.Accepted)
+    }
+
+    function doneRejected() {
+        dialog.done(ColorDialog.Rejected)
+    }
+
+    ColorDialog {
+        id: dialog
+        objectName: "ColorDialog"
+    }
+}
diff --git a/tests/auto/quickdialogs/qquickcolordialogimpl/data/colorDialogWithoutWindowVisibleTrue.qml b/tests/auto/quickdialogs/qquickcolordialogimpl/data/colorDialogWithoutWindowVisibleTrue.qml
new file mode 100644
index 0000000000..b146d61513
--- /dev/null
+++ b/tests/auto/quickdialogs/qquickcolordialogimpl/data/colorDialogWithoutWindowVisibleTrue.qml
@@ -0,0 +1,28 @@
+// Copyright (C) 2023 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR BSD-3-Clause
+
+import QtQuick
+import QtQuick.Controls
+import QtQuick.Dialogs
+
+Rectangle {
+    width: 480
+    height: 640
+    color: dialog.selectedColor
+
+    property alias dialog: dialog
+
+    function doneAccepted() {
+        dialog.done(ColorDialog.Accepted)
+    }
+
+    function doneRejected() {
+        dialog.done(ColorDialog.Rejected)
+    }
+
+    ColorDialog {
+        id: dialog
+        objectName: "ColorDialog"
+	visible: true
+    }
+}
diff --git a/tests/auto/quickdialogs/qquickcolordialogimpl/data/windowSwapping.qml b/tests/auto/quickdialogs/qquickcolordialogimpl/data/windowSwapping.qml
new file mode 100644
index 0000000000..5215feb243
--- /dev/null
+++ b/tests/auto/quickdialogs/qquickcolordialogimpl/data/windowSwapping.qml
@@ -0,0 +1,58 @@
+// Copyright (C) 2022 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR BSD-3-Clause
+
+import QtQuick
+import QtQuick.Controls
+import QtQuick.Dialogs
+
+Window {
+    width: 480
+    height: 640
+
+    property alias dialog: dialog
+
+    function getSubWindow1 () {
+        return subwindow1
+    }
+
+    function getSubWindow2 () {
+        return subwindow2
+    }
+
+    function goToSubWindow1() {
+        dialog.close()
+        dialog.parentWindow = subwindow1
+        dialog.open()
+    }
+
+    function goToSubWindow2() {
+        dialog.close()
+        dialog.parentWindow = subwindow2
+        dialog.open()
+    }
+
+    function resetParentWindow() {
+        dialog.close()
+        dialog.parentWindow = undefined
+        dialog.open()
+    }
+
+    Window {
+        id: subwindow1
+        width: 480
+        height: 640
+        visible: true
+    }
+
+    Window {
+        id: subwindow2
+        width: 480
+        height: 640
+        visible: true
+    }
+
+    ColorDialog {
+        id: dialog
+        objectName: "ColorDialog"
+    }
+}
diff --git a/tests/auto/quickdialogs/qquickcolordialogimpl/tst_qquickcolordialogimpl.cpp b/tests/auto/quickdialogs/qquickcolordialogimpl/tst_qquickcolordialogimpl.cpp
index 23f1e6844b..c93ef3d12f 100644
--- a/tests/auto/quickdialogs/qquickcolordialogimpl/tst_qquickcolordialogimpl.cpp
+++ b/tests/auto/quickdialogs/qquickcolordialogimpl/tst_qquickcolordialogimpl.cpp
@@ -4,6 +4,7 @@
 #include <QtTest/qtest.h>
 #include <QtQuickTest/quicktest.h>
 #include <QtTest/qsignalspy.h>
+#include <QtQuick/qquickview.h>
 #include <QtQuick/private/qquicklistview_p.h>
 #include <QtQuickControlsTestUtils/private/controlstestutils_p.h>
 #include <QtQuickControlsTestUtils/private/dialogstestutils_p.h>
@@ -53,6 +54,9 @@ private slots:
     void changeColorFromTextFields();
     void windowTitle_data();
     void windowTitle();
+    void workingInsideQQuickViewer_data();
+    void workingInsideQQuickViewer();
+    void dialogCanMoveBetweenWindows();
 
 private:
     bool closePopup(DialogTestHelper<QQuickColorDialog, QQuickColorDialogImpl> *dialogHelper,
@@ -456,6 +460,60 @@ void tst_QQuickColorDialogImpl::windowTitle()
     CLOSE_DIALOG("Ok");
 }
 
+void tst_QQuickColorDialogImpl::workingInsideQQuickViewer_data()
+{
+    QTest::addColumn<bool>("initialVisible");
+    QTest::addColumn<QString>("urlToQmlFile");
+    QTest::newRow("visible: false") << false << "colorDialogWithoutWindow.qml";
+    QTest::newRow("visible: true") << true << "colorDialogWithoutWindowVisibleTrue.qml";
+}
+
+void tst_QQuickColorDialogImpl::workingInsideQQuickViewer() // QTBUG-106598
+{
+    QFETCH(bool, initialVisible);
+    QFETCH(QString, urlToQmlFile);
+
+    QQuickView dialogView;
+    dialogView.setSource(testFileUrl(urlToQmlFile));
+    dialogView.show();
+
+    auto dialog = dialogView.findChild<QQuickColorDialog *>("ColorDialog");
+    QVERIFY(dialog);
+    QCOMPARE(dialog->isVisible(), initialVisible);
+
+    dialog->open();
+    QVERIFY(dialog->isVisible());
+}
+
+void tst_QQuickColorDialogImpl::dialogCanMoveBetweenWindows()
+{
+    DialogTestHelper<QQuickColorDialog, QQuickColorDialogImpl> dialogHelper(this, "windowSwapping.qml");
+    QVERIFY2(dialogHelper.isWindowInitialized(), dialogHelper.failureMessage());
+    QVERIFY(dialogHelper.waitForWindowActive());
+
+    QVERIFY(dialogHelper.openDialog());
+    QTRY_VERIFY(dialogHelper.isQuickDialogOpen());
+
+    QCOMPARE(dialogHelper.quickDialog->parent(), dialogHelper.window());
+    QVariant subWindow1;
+    QVariant subWindow2;
+
+    QMetaObject::invokeMethod(dialogHelper.window(), "getSubWindow1", Q_RETURN_ARG(QVariant, subWindow1));
+    QMetaObject::invokeMethod(dialogHelper.window(), "getSubWindow2", Q_RETURN_ARG(QVariant, subWindow2));
+
+    // Confirm that the dialog can swap to different windows
+    QMetaObject::invokeMethod(dialogHelper.window(), "goToSubWindow1");
+    QCOMPARE(dialogHelper.dialog->parentWindow(), qvariant_cast<QQuickWindow *>(subWindow1));
+
+    QMetaObject::invokeMethod(dialogHelper.window(), "goToSubWindow2");
+    QCOMPARE(dialogHelper.dialog->parentWindow(), qvariant_cast<QQuickWindow *>(subWindow2));
+
+    QMetaObject::invokeMethod(dialogHelper.window(), "resetParentWindow");
+    QCOMPARE(dialogHelper.quickDialog->parent(), dialogHelper.window());
+
+    CLOSE_DIALOG("Ok");
+}
+
 QTEST_MAIN(tst_QQuickColorDialogImpl)
 
 #include "tst_qquickcolordialogimpl.moc"