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 6.6 6.7
Change-Id: Idd7c0ecdeea6cbf26e8d41168788563ee9794118
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>

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 (dialogCode() == 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;
+
+    m_visibleRequested = false;
+
+    if (windowForOpen()) {
+        open();
+        return;
     }
 
-    if (m_visibleRequested) {
-        qCDebug(lcDialogs) << "visible was bound to true before component completion; opening dialog";
-        open();
-        m_visibleRequested = false;
-    }
+    // 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)
@@ -463,21 +485,33 @@ void QQuickAbstractDialog::onHide(QPlatformDialogHelper *dialog)
 
 int QQuickAbstractDialog::dialogCode() const { return m_result; }
 
-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"

src/quickdialogs/quickdialogs/qquickabstractdialog_p.h

@@ -37,7 +37,7 @@ class Q_QUICKDIALOGS2_EXPORT QQuickAbstractDialog : public QObject, public QQmlP
     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);
@@ -107,12 +108,10 @@ protected:
     virtual void onHide(QPlatformDialogHelper *dialog);
     virtual int dialogCode() const;
 
-    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;
     int m_result = Rejected;
     QWindow *m_parentWindow = nullptr;
     QString m_title;
@@ -121,6 +120,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

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"
+    }
+}

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
+    }
+}

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"
+    }
+}

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>
@@ -54,6 +55,9 @@ private slots:
     void changeColorFromTextFields();
     void windowTitle_data();
     void windowTitle();
+    void workingInsideQQuickViewer_data();
+    void workingInsideQQuickViewer();
+    void dialogCanMoveBetweenWindows();
 
 private:
     bool closePopup(DialogTestHelper<QQuickColorDialog, QQuickColorDialogImpl> *dialogHelper,
@@ -474,6 +478,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"