Close QDialog via QWidget::close()

By going via QWidget::close() we ensure that if there's a QWidgetWindow
backing the dialog (which is almost always the case), we will plumb down
to QWindow::close(), resulting in QEvent::Close events to the QWindow.

Since we don't want QDialog subclasses to receive a call to a closeEvent
override that they didn't receive before (and which they might interpret
as rejection or cancellation), install a temporary event filter that
eats the QCloseEvent resulting from the call to close().

Task-number: QTBUG-53286
Change-Id: Ie8f6f0cb3160acfd5865dc74f0a7b6d87f838724
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>

4 files changed, 31 insertions, 10 deletions

diff --git a/src/widgets/dialogs/qdialog.cpp b/src/widgets/dialogs/qdialog.cpp
index 9e18e2443a..9f4e95d83c 100644
--- a/src/widgets/dialogs/qdialog.cpp
+++ b/src/widgets/dialogs/qdialog.cpp
@@ -146,16 +146,33 @@ bool QDialogPrivate::canBeNativeDialog() const
 /*!
     \internal
 
-    Properly hides dialog and sets the \a resultCode.
+    Properly closes dialog and sets the \a resultCode.
  */
-void QDialogPrivate::hide(int resultCode)
+void QDialogPrivate::close(int resultCode)
 {
     Q_Q(QDialog);
 
     q->setResult(resultCode);
     q->hide();
+    if (!data.is_closing) {
+        // Until Qt 6.3 we didn't close dialogs, so they didn't receive a QCloseEvent.
+        // It is likely that subclasses implement closeEvent and handle them as rejection
+        // (like QMessageBox and QProgressDialog do), so eat those events.
+        struct CloseEventEater : QObject
+        {
+            using QObject::QObject;
+        protected:
+            bool eventFilter(QObject *o, QEvent *e) override
+            {
+                if (e->type() == QEvent::Close)
+                    return true;
+                return QObject::eventFilter(o, e);
+            }
+        } closeEventEater;
+        q->installEventFilter(&closeEventEater);
+        QWidgetPrivate::close();
+    }
 
-    handleClose(QWidgetPrivate::CloseNoEvent);
     resetModalitySetByOpen();
 }
 
@@ -632,7 +649,7 @@ int QDialog::exec()
 void QDialog::done(int r)
 {
     Q_D(QDialog);
-    d->hide(r);
+    d->close(r);
     d->finalize(r, r);
 }
 
diff --git a/src/widgets/dialogs/qdialog_p.h b/src/widgets/dialogs/qdialog_p.h
index 0c31593ced..7c7e8f89a7 100644
--- a/src/widgets/dialogs/qdialog_p.h
+++ b/src/widgets/dialogs/qdialog_p.h
@@ -122,7 +122,7 @@ public:
     QPlatformDialogHelper *platformHelper() const;
     virtual bool canBeNativeDialog() const;
 
-    void hide(int resultCode);
+    void close(int resultCode);
     void finalize(int resultCode, int dialogCode);
 
 private:
diff --git a/src/widgets/dialogs/qmessagebox.cpp b/src/widgets/dialogs/qmessagebox.cpp
index ac2f8bd2d5..a3093f82c2 100644
--- a/src/widgets/dialogs/qmessagebox.cpp
+++ b/src/widgets/dialogs/qmessagebox.cpp
@@ -518,7 +518,7 @@ void QMessageBoxPrivate::setClickedButton(QAbstractButton *button)
     emit q->buttonClicked(clickedButton);
 
     auto resultCode = execReturnCode(button);
-    hide(resultCode);
+    close(resultCode);
     finalize(resultCode, dialogCodeForButton(button));
 }
 
@@ -1420,8 +1420,10 @@ void QMessageBox::closeEvent(QCloseEvent *e)
         return;
     }
     QDialog::closeEvent(e);
-    d->clickedButton = d->detectedEscapeButton;
-    setResult(d->execReturnCode(d->detectedEscapeButton));
+    if (!d->clickedButton) {
+        d->clickedButton = d->detectedEscapeButton;
+        setResult(d->execReturnCode(d->detectedEscapeButton));
+    }
 }
 
 /*!
diff --git a/tests/auto/widgets/dialogs/qdialog/tst_qdialog.cpp b/tests/auto/widgets/dialogs/qdialog/tst_qdialog.cpp
index 1dd053fa3f..a509f1135a 100644
--- a/tests/auto/widgets/dialogs/qdialog/tst_qdialog.cpp
+++ b/tests/auto/widgets/dialogs/qdialog/tst_qdialog.cpp
@@ -615,7 +615,9 @@ void tst_QDialog::virtualsOnClose()
         dialog.show();
         QVERIFY(QTest::qWaitForWindowExposed(&dialog));
         dialog.accept();
-        QCOMPARE(dialog.closeEventCount, 0); // we only hide the dialog
+        // we used to only hide the dialog, and we still don't want a
+        // closeEvent call for application-triggered calls to QDialog::done
+        QCOMPARE(dialog.closeEventCount, 0);
         QCOMPARE(dialog.acceptCount, 1);
         QCOMPARE(dialog.rejectCount, 0);
         QCOMPARE(dialog.doneCount, 1);
@@ -626,7 +628,7 @@ void tst_QDialog::virtualsOnClose()
         dialog.show();
         QVERIFY(QTest::qWaitForWindowExposed(&dialog));
         dialog.reject();
-        QCOMPARE(dialog.closeEventCount, 0); // we only hide the dialog
+        QCOMPARE(dialog.closeEventCount, 0);
         QCOMPARE(dialog.acceptCount, 0);
         QCOMPARE(dialog.rejectCount, 1);
         QCOMPARE(dialog.doneCount, 1);