QMessageBox: Respect clients overriding QDialog::done()

As a result of d8bbb5ee0e60d44a70d29306e607a59caf7fe5bc, we
were no longer calling QDialog::done(), which users may have
overridden.

We now pull out the dialog code to determine whether to
emit accepted/rejected directly in done(), so that we
can go back to calling QDialog::done().

Change-Id: Ie08270123d61d9010acd8c989b66986f71960ad0
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
(cherry picked from commit 1f70c073d4325bc0eb9b0cec5156c3b89ce1b4df)
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>

src/widgets/dialogs/qdialog.cpp

@@ -149,27 +149,6 @@ void QDialogPrivate::close(int resultCode)
     resetModalitySetByOpen();
 }
 
-/*!
-    \internal
-
-    Emits finished() signal with \a resultCode. If the \a dialogCode
-    is equal to 0 emits rejected(), if the \a dialogCode is equal to
-    1 emits accepted().
- */
-void QDialogPrivate::finalize(int resultCode, int dialogCode)
-{
-    Q_Q(QDialog);
-    QPointer<QDialog> guard(q);
-
-    if (dialogCode == QDialog::Accepted)
-        emit q->accepted();
-    else if (dialogCode == QDialog::Rejected)
-        emit q->rejected();
-
-    if (guard)
-        emit q->finished(resultCode);
-}
-
 QWindow *QDialogPrivate::transientParentWindow() const
 {
     Q_Q(const QDialog);
@@ -618,9 +597,22 @@ int QDialog::exec()
 
 void QDialog::done(int r)
 {
+    QPointer<QDialog> guard(this);
+
     Q_D(QDialog);
     d->close(r);
-    d->finalize(r, r);
+
+    if (!guard)
+        return;
+
+    int dialogCode = d->dialogCode();
+    if (dialogCode == QDialog::Accepted)
+        emit accepted();
+    else if (dialogCode == QDialog::Rejected)
+        emit rejected();
+
+    if (guard)
+        emit finished(r);
 }
 
 /*!

src/widgets/dialogs/qdialog_p.h

@@ -88,7 +88,9 @@ public:
     virtual bool canBeNativeDialog() const;
 
     void close(int resultCode);
-    void finalize(int resultCode, int dialogCode);
+
+protected:
+    virtual int dialogCode() const { return rescode; }
 
 private:
     virtual void initHelper(QPlatformDialogHelper *) {}

src/widgets/dialogs/qmessagebox.cpp

@@ -187,8 +187,6 @@ public:
     QAbstractButton *abstractButtonForId(int id) const;
     int execReturnCode(QAbstractButton *button);
 
-    int dialogCodeForButtonRole(QMessageBox::ButtonRole buttonRole) const;
-
     void detectEscapeButton();
     void updateSize();
     int layoutMinimumWidth();
@@ -239,6 +237,7 @@ public:
 private:
     void initHelper(QPlatformDialogHelper *) override;
     void helperPrepareShow(QPlatformDialogHelper *) override;
+    int dialogCode() const override;
 };
 
 void QMessageBoxPrivate::init(const QString &title, const QString &text)
@@ -442,23 +441,24 @@ int QMessageBoxPrivate::execReturnCode(QAbstractButton *button)
     return ret;
 }
 
-/*!
-    \internal
-
-    Returns 0 for RejectedRole and NoRole, 1 for AcceptedRole and YesRole, -1 otherwise
- */
-int QMessageBoxPrivate::dialogCodeForButtonRole(QMessageBox::ButtonRole buttonRole) const
+int QMessageBoxPrivate::dialogCode() const
 {
-    switch (buttonRole) {
-    case QMessageBox::AcceptRole:
-    case QMessageBox::YesRole:
-        return QDialog::Accepted;
-    case QMessageBox::RejectRole:
-    case QMessageBox::NoRole:
-        return QDialog::Rejected;
-    default:
-        return -1;
+    Q_Q(const QMessageBox);
+
+    if (clickedButton) {
+        switch (q->buttonRole(clickedButton)) {
+        case QMessageBox::AcceptRole:
+        case QMessageBox::YesRole:
+            return QDialog::Accepted;
+        case QMessageBox::RejectRole:
+        case QMessageBox::NoRole:
+            return QDialog::Rejected;
+        default:
+            ;
+        }
     }
+
+    return QDialogPrivate::dialogCode();
 }
 
 void QMessageBoxPrivate::_q_buttonClicked(QAbstractButton *button)
@@ -492,8 +492,7 @@ void QMessageBoxPrivate::setClickedButton(QAbstractButton *button)
     emit q->buttonClicked(clickedButton);
 
     auto resultCode = execReturnCode(button);
-    close(resultCode);
-    finalize(resultCode, dialogCodeForButtonRole(q->buttonRole(button)));
+    q->done(resultCode);
 }
 
 void QMessageBoxPrivate::_q_helperClicked(QPlatformDialogHelper::StandardButton helperButton, QPlatformDialogHelper::ButtonRole role)

tests/auto/widgets/dialogs/qmessagebox/tst_qmessagebox.cpp

@@ -58,6 +58,9 @@ private slots:
     void acceptedRejectedSignals();
     void acceptedRejectedSignals_data();
 
+    void overrideDone_data();
+    void overrideDone();
+
     void cleanup();
 };
 
@@ -152,6 +155,44 @@ void tst_QMessageBox::init()
     qApp->setAttribute(Qt::AA_DontUseNativeDialogs, !useNativeDialog);
 }
 
+class OverridingMessageBox : public QMessageBox
+{
+public:
+    void done(int result) override {
+        doneResult = result;
+        QMessageBox::done(result);
+    }
+    std::optional<int> doneResult;
+};
+
+void tst_QMessageBox::overrideDone_data()
+{
+    QTest::addColumn<QMessageBox::StandardButton>("button");
+    QTest::addColumn<int>("closeAction");
+    QTest::addColumn<int>("result");
+
+    QTest::newRow("close") << QMessageBox::Help << int(ExecCloseHelper::CloseWindow) << 0;
+    QTest::newRow("yes") << QMessageBox::Yes << int(Qt::Key_Enter) << int(QMessageBox::Yes);
+    QTest::newRow("no") << QMessageBox::No << int(Qt::Key_Enter) << int(QMessageBox::No);
+}
+
+void tst_QMessageBox::overrideDone()
+{
+    QFETCH(QMessageBox::StandardButton, button);
+    QFETCH(int, closeAction);
+    QFETCH(int, result);
+
+    OverridingMessageBox messageBox;
+    messageBox.addButton(button);
+    messageBox.setDefaultButton(button);
+    ExecCloseHelper closeHelper;
+    closeHelper.start(closeAction, &messageBox);
+    messageBox.exec();
+    QVERIFY(messageBox.doneResult.has_value());
+    QCOMPARE(*messageBox.doneResult, result);
+
+}
+
 void tst_QMessageBox::cleanup()
 {
     QTRY_VERIFY(QApplication::topLevelWidgets().isEmpty()); // OS X requires TRY