Dialogs: clean up native dialogs when object gets destroyed

QWidget::setVisible is virtual, and called via hide() by both the
QDialog and the QWidget destructor. A dialog that becomes invisible
when getting destroyed will at most execute the QDialog override.
Subclassing QDialog and overriding setVisible() to update the state
of the native platform dialog will not work, unless we explicitly
call hide() in the respective subclass's destructor.

Since e0bb9e81ab1a9d71f2893844ea82, QDialogPrivate::setVisible is
also virtual, and gets called by QDialog::setVisible. So the clean
solution is to move the implementation of the native dialog status
update into an override of QDialogPrivate::setVisible.

Add test that verifies that the transient parent of the dialog
becomes inactive when the (native) dialog shows (and skip if that
fails), and then becomes active again when the (native) dialog is
closed through the destructor of the Q*Dialog class. The test of
QFileDialog has to be skipped on Android for the same reason as the
widgetlessNativeDialog.

Fixes: QTBUG-116277
Pick-to: 6.6 6.5
Change-Id: Ie3f93980d8653b8d933bf70aac3ef90de606f0ef
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>

5 files changed, 76 insertions, 37 deletions

diff --git a/src/widgets/dialogs/qcolordialog.cpp b/src/widgets/dialogs/qcolordialog.cpp
index 8b3fb28c7d..5ec2424b5c 100644
--- a/src/widgets/dialogs/qcolordialog.cpp
+++ b/src/widgets/dialogs/qcolordialog.cpp
@@ -120,6 +120,7 @@ public:
     bool handleColorPickingKeyPress(QKeyEvent *e);
 
     bool canBeNativeDialog() const override;
+    void setVisible(bool visible) override;
 
     QWellArray *custom;
     QWellArray *standard;
@@ -2139,30 +2140,42 @@ QColorDialog::ColorDialogOptions QColorDialog::options() const
 */
 void QColorDialog::setVisible(bool visible)
 {
-    Q_D(QColorDialog);
+    // will call QColorDialogPrivate::setVisible override
+    QDialog::setVisible(visible);
+}
 
+/*!
+    \internal
+
+    The implementation of QColorDialog::setVisible() has to live here so that the call
+    to hide() in ~QDialog calls this function; it wouldn't call the override of
+    QDialog::setVisible().
+*/
+void QColorDialogPrivate::setVisible(bool visible)
+{
+    Q_Q(QColorDialog);
     if (visible){
-        if (testAttribute(Qt::WA_WState_ExplicitShowHide) && !testAttribute(Qt::WA_WState_Hidden))
+        if (q->testAttribute(Qt::WA_WState_ExplicitShowHide) && !q->testAttribute(Qt::WA_WState_Hidden))
             return;
-    } else if (testAttribute(Qt::WA_WState_ExplicitShowHide) && testAttribute(Qt::WA_WState_Hidden))
+    } else if (q->testAttribute(Qt::WA_WState_ExplicitShowHide) && q->testAttribute(Qt::WA_WState_Hidden))
         return;
 
     if (visible)
-        d->selectedQColor = QColor();
+        selectedQColor = QColor();
 
-    if (d->nativeDialogInUse) {
-        if (d->setNativeDialogVisible(visible)) {
+    if (nativeDialogInUse) {
+        if (setNativeDialogVisible(visible)) {
             // Set WA_DontShowOnScreen so that QDialog::setVisible(visible) below
             // updates the state correctly, but skips showing the non-native version:
-            setAttribute(Qt::WA_DontShowOnScreen);
+            q->setAttribute(Qt::WA_DontShowOnScreen);
         } else {
-            d->initWidgets();
+            initWidgets();
         }
     } else {
-        setAttribute(Qt::WA_DontShowOnScreen, false);
+        q->setAttribute(Qt::WA_DontShowOnScreen, false);
     }
 
-    QDialog::setVisible(visible);
+    QDialogPrivate::setVisible(visible);
 }
 
 /*!
@@ -2210,7 +2223,6 @@ QColor QColorDialog::getColor(const QColor &initial, QWidget *parent, const QStr
 
 QColorDialog::~QColorDialog()
 {
-
 }
 
 /*!
diff --git a/src/widgets/dialogs/qfiledialog.cpp b/src/widgets/dialogs/qfiledialog.cpp
index a07cc33711..e985baa3af 100644
--- a/src/widgets/dialogs/qfiledialog.cpp
+++ b/src/widgets/dialogs/qfiledialog.cpp
@@ -835,41 +835,53 @@ void QFileDialog::open(QObject *receiver, const char *member)
 */
 void QFileDialog::setVisible(bool visible)
 {
-    Q_D(QFileDialog);
+    // will call QFileDialogPrivate::setVisible override
+    QDialog::setVisible(visible);
+}
+
+/*!
+    \internal
+
+    The logic has to live here so that the call to hide() in ~QDialog calls
+    this function; it wouldn't call an override of QDialog::setVisible().
+*/
+void QFileDialogPrivate::setVisible(bool visible)
+{
+    Q_Q(QFileDialog);
     if (visible){
-        if (testAttribute(Qt::WA_WState_ExplicitShowHide) && !testAttribute(Qt::WA_WState_Hidden))
+        if (q->testAttribute(Qt::WA_WState_ExplicitShowHide) && !q->testAttribute(Qt::WA_WState_Hidden))
             return;
-    } else if (testAttribute(Qt::WA_WState_ExplicitShowHide) && testAttribute(Qt::WA_WState_Hidden))
+    } else if (q->testAttribute(Qt::WA_WState_ExplicitShowHide) && q->testAttribute(Qt::WA_WState_Hidden))
         return;
 
-    if (d->canBeNativeDialog()){
-        if (d->setNativeDialogVisible(visible)){
-            // Set WA_DontShowOnScreen so that QDialog::setVisible(visible) below
+    if (canBeNativeDialog()){
+        if (setNativeDialogVisible(visible)){
+            // Set WA_DontShowOnScreen so that QDialogPrivate::setVisible(visible) below
             // updates the state correctly, but skips showing the non-native version:
-            setAttribute(Qt::WA_DontShowOnScreen);
+            q->setAttribute(Qt::WA_DontShowOnScreen);
 #if QT_CONFIG(fscompleter)
             // So the completer doesn't try to complete and therefore show a popup
-            if (!d->nativeDialogInUse)
-                d->completer->setModel(nullptr);
+            if (!nativeDialogInUse)
+                completer->setModel(nullptr);
 #endif
         } else {
-            d->createWidgets();
-            setAttribute(Qt::WA_DontShowOnScreen, false);
+            createWidgets();
+            q->setAttribute(Qt::WA_DontShowOnScreen, false);
 #if QT_CONFIG(fscompleter)
-            if (!d->nativeDialogInUse) {
-                if (d->proxyModel != nullptr)
-                    d->completer->setModel(d->proxyModel);
+            if (!nativeDialogInUse) {
+                if (proxyModel != nullptr)
+                    completer->setModel(proxyModel);
                 else
-                    d->completer->setModel(d->model);
+                    completer->setModel(model);
             }
 #endif
         }
     }
 
-    if (visible && d->usingWidgets())
-        d->qFileDialogUi->fileNameEdit->setFocus();
+    if (visible && usingWidgets())
+        qFileDialogUi->fileNameEdit->setFocus();
 
-    QDialog::setVisible(visible);
+    QDialogPrivate::setVisible(visible);
 }
 
 /*!
diff --git a/src/widgets/dialogs/qfiledialog_p.h b/src/widgets/dialogs/qfiledialog_p.h
index dc5e33d7e4..0067a90061 100644
--- a/src/widgets/dialogs/qfiledialog_p.h
+++ b/src/widgets/dialogs/qfiledialog_p.h
@@ -221,6 +221,7 @@ public:
     // dialog. Returning false means that a non-native dialog must be
     // used instead.
     bool canBeNativeDialog() const override;
+    void setVisible(bool visible) override;
     inline bool usingWidgets() const;
 
     inline void setDirectory_sys(const QUrl &directory);
diff --git a/src/widgets/dialogs/qfontdialog.cpp b/src/widgets/dialogs/qfontdialog.cpp
index 45853e153f..855376292b 100644
--- a/src/widgets/dialogs/qfontdialog.cpp
+++ b/src/widgets/dialogs/qfontdialog.cpp
@@ -960,19 +960,32 @@ void QFontDialog::open(QObject *receiver, const char *member)
 */
 void QFontDialog::setVisible(bool visible)
 {
-    if (testAttribute(Qt::WA_WState_ExplicitShowHide) && testAttribute(Qt::WA_WState_Hidden) != visible)
+    // will call QFontDialogPrivate::setVisible
+    QDialog::setVisible(visible);
+}
+
+/*!
+    \internal
+
+    The implementation of QFontDialog::setVisible() has to live here so that the call
+    to hide() in ~QDialog calls this function; it wouldn't call the override of
+    QDialog::setVisible().
+*/
+void QFontDialogPrivate::setVisible(bool visible)
+{
+    Q_Q(QFontDialog);
+    if (q->testAttribute(Qt::WA_WState_ExplicitShowHide) && q->testAttribute(Qt::WA_WState_Hidden) != visible)
         return;
-    Q_D(QFontDialog);
-    if (d->canBeNativeDialog())
-        d->setNativeDialogVisible(visible);
-    if (d->nativeDialogInUse) {
+    if (canBeNativeDialog())
+        setNativeDialogVisible(visible);
+    if (nativeDialogInUse) {
         // Set WA_DontShowOnScreen so that QDialog::setVisible(visible) below
         // updates the state correctly, but skips showing the non-native version:
-        setAttribute(Qt::WA_DontShowOnScreen, true);
+        q->setAttribute(Qt::WA_DontShowOnScreen, true);
     } else {
-        setAttribute(Qt::WA_DontShowOnScreen, false);
+        q->setAttribute(Qt::WA_DontShowOnScreen, false);
     }
-    QDialog::setVisible(visible);
+    QDialogPrivate::setVisible(visible);
 }
 
 /*!
diff --git a/src/widgets/dialogs/qfontdialog_p.h b/src/widgets/dialogs/qfontdialog_p.h
index 4b3b45a9be..d46d1986e8 100644
--- a/src/widgets/dialogs/qfontdialog_p.h
+++ b/src/widgets/dialogs/qfontdialog_p.h
@@ -105,6 +105,7 @@ public:
     QByteArray memberToDisconnectOnClose;
 
     bool canBeNativeDialog() const override;
+    void setVisible(bool visible) override;
     void _q_runNativeAppModalPanel();
 
 private: