Respect roles of buttons added to QMessageBox

When a button added to QMessageBox has AcceptRole or YesRole, the signal
accepted() will be emitted upon click on the button. If the button has
RejectRole or NoRole, the signal rejected() will be emitted upon click
on the button. If a button has a different role, neither accepted() nor
rejected() will be emitted. This works for both standard and custom
buttons.
The signal finished() with result code will be sent regardless of a
clicked button role.
Also added documentation strings for some methods of private classes in
order to have better tooltips in IDE(s).

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

src/widgets/dialogs/qdialog.cpp

@@ -145,6 +145,41 @@ bool QDialogPrivate::canBeNativeDialog() const
     return false;
 }
 
+/*!
+    \internal
+
+    Properly hides dialog and sets the \p resultCode
+ */
+void QDialogPrivate::hide(int resultCode)
+{
+    Q_Q(QDialog);
+
+    q->setResult(resultCode);
+    q->hide();
+
+    close_helper(QWidgetPrivate::CloseNoEvent);
+    resetModalitySetByOpen();
+}
+
+/*!
+    \internal
+
+    Emits finished() signal with \p resultCode. If the \p dialogCode
+    is equal to 0 emits rejected(), if the \p dialogCode is equal to
+    1 emits accepted().
+ */
+void QDialogPrivate::finalize(int resultCode, int dialogCode)
+{
+    Q_Q(QDialog);
+
+    if (dialogCode == QDialog::Accepted)
+        emit q->accepted();
+    else if (dialogCode == QDialog::Rejected)
+        emit q->rejected();
+
+    emit q->finished(resultCode);
+}
+
 QWindow *QDialogPrivate::transientParentWindow() const
 {
     Q_Q(const QDialog);
@@ -593,17 +628,8 @@ int QDialog::exec()
 void QDialog::done(int r)
 {
     Q_D(QDialog);
-    setResult(r);
-    hide();
-
-    d->close_helper(QWidgetPrivate::CloseNoEvent);
-    d->resetModalitySetByOpen();
-
-    emit finished(r);
-    if (r == Accepted)
-        emit accepted();
-    else if (r == Rejected)
-        emit rejected();
+    d->hide(r);
+    d->finalize(r, r);
 }
 
 /*!

src/widgets/dialogs/qdialog_p.h

@@ -122,6 +122,9 @@ public:
     QPlatformDialogHelper *platformHelper() const;
     virtual bool canBeNativeDialog() const;
 
+    void hide(int resultCode);
+    void finalize(int resultCode, int dialogCode);
+
 private:
     virtual void initHelper(QPlatformDialogHelper *) {}
     virtual void helperPrepareShow(QPlatformDialogHelper *) {}

src/widgets/dialogs/qmessagebox.cpp

@@ -209,6 +209,7 @@ public:
     void setupLayout();
     void _q_buttonClicked(QAbstractButton *);
     void _q_clicked(QPlatformDialogHelper::StandardButton button, QPlatformDialogHelper::ButtonRole role);
+    void setClickedButton(QAbstractButton *button);
 
     QAbstractButton *findButton(int button0, int button1, int button2, int flags);
     void addOldButtons(int button0, int button1, int button2);
@@ -216,6 +217,8 @@ public:
     QAbstractButton *abstractButtonForId(int id) const;
     int execReturnCode(QAbstractButton *button);
 
+    int dialogCodeForButton(QAbstractButton *button) const;
+
     void detectEscapeButton();
     void updateSize();
     int layoutMinimumWidth();
@@ -466,6 +469,27 @@ int QMessageBoxPrivate::execReturnCode(QAbstractButton *button)
     return ret;
 }
 
+/*!
+    \internal
+
+    Returns 0 for RejectedRole and NoRole, 1 for AcceptedRole and YesRole, -1 otherwise
+ */
+int QMessageBoxPrivate::dialogCodeForButton(QAbstractButton *button) const
+{
+    Q_Q(const QMessageBox);
+
+    switch (q->buttonRole(button)) {
+    case QMessageBox::AcceptRole:
+    case QMessageBox::YesRole:
+        return QDialog::Accepted;
+    case QMessageBox::RejectRole:
+    case QMessageBox::NoRole:
+        return QDialog::Rejected;
+    default:
+        return -1;
+    }
+}
+
 void QMessageBoxPrivate::_q_buttonClicked(QAbstractButton *button)
 {
     Q_Q(QMessageBox);
@@ -477,20 +501,30 @@ void QMessageBoxPrivate::_q_buttonClicked(QAbstractButton *button)
     } else
 #endif
     {
-        clickedButton = button;
-        q->done(execReturnCode(button)); // does not trigger closeEvent
-        emit q->buttonClicked(button);
+        setClickedButton(button);
 
         if (receiverToDisconnectOnClose) {
             QObject::disconnect(q, signalToDisconnectOnClose, receiverToDisconnectOnClose,
                                 memberToDisconnectOnClose);
-            receiverToDisconnectOnClose = 0;
+            receiverToDisconnectOnClose = nullptr;
         }
         signalToDisconnectOnClose.clear();
         memberToDisconnectOnClose.clear();
     }
 }
 
+void QMessageBoxPrivate::setClickedButton(QAbstractButton *button)
+{
+    Q_Q(QMessageBox);
+
+    clickedButton = button;
+    emit q->buttonClicked(clickedButton);
+
+    auto resultCode = execReturnCode(button);
+    hide(resultCode);
+    finalize(resultCode, dialogCodeForButton(button));
+}
+
 void QMessageBoxPrivate::_q_clicked(QPlatformDialogHelper::StandardButton button, QPlatformDialogHelper::ButtonRole role)
 {
     Q_Q(QMessageBox);

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

@@ -110,6 +110,10 @@ private slots:
     void setInformativeText();
     void iconPixmap();
 
+    // QTBUG-44131
+    void acceptedRejectedSignals();
+    void acceptedRejectedSignals_data();
+
     void cleanup();
 };
 
@@ -717,5 +721,96 @@ void tst_QMessageBox::iconPixmap()
     QCOMPARE(messageBox.iconPixmap(), QPixmap());
 }
 
+using SignalSignature = void(QDialog::*)();
+Q_DECLARE_METATYPE(SignalSignature);
+Q_DECLARE_METATYPE(QMessageBox::ButtonRole)
+
+using ButtonsCreator = std::function<QVector<QPushButton*>(QMessageBox &)>;
+Q_DECLARE_METATYPE(ButtonsCreator);
+
+using RoleSet = QSet<QMessageBox::ButtonRole>;
+Q_DECLARE_METATYPE(RoleSet);
+
+void tst_QMessageBox::acceptedRejectedSignals()
+{
+    QMessageBox messageBox(QMessageBox::Information, "Test window", "Test text");
+
+    QFETCH(ButtonsCreator, buttonsCreator);
+    QVERIFY(buttonsCreator);
+
+    const auto buttons = buttonsCreator(messageBox);
+    QVERIFY(!buttons.isEmpty());
+
+    QFETCH(RoleSet, roles);
+    QFETCH(SignalSignature, signalSignature);
+    for (auto button: buttons) {
+        QVERIFY(button);
+
+        messageBox.show();
+        QVERIFY(QTest::qWaitForWindowExposed(&messageBox));
+
+        QSignalSpy spy(&messageBox, signalSignature);
+        QVERIFY(spy.isValid());
+        button->click();
+
+        if (roles.contains(messageBox.buttonRole(button)))
+            QCOMPARE(spy.count(), 1);
+        else
+            QVERIFY(spy.isEmpty());
+    }
+}
+
+static void addAcceptedRow(const char *title, ButtonsCreator bc)
+{
+    QTest::newRow(title) << (RoleSet() << QMessageBox::AcceptRole << QMessageBox::YesRole)
+                         << &QDialog::accepted << bc;
+}
+
+static void addRejectedRow(const char *title, ButtonsCreator bc)
+{
+    QTest::newRow(title) << (RoleSet() << QMessageBox::RejectRole << QMessageBox::NoRole)
+                         << &QDialog::rejected << bc;
+}
+
+static void addCustomButtonsData()
+{
+    ButtonsCreator buttonsCreator = [](QMessageBox &messageBox) {
+        QVector<QPushButton*> buttons(QMessageBox::NRoles);
+        for (int i = QMessageBox::AcceptRole; i < QMessageBox::NRoles; ++i) {
+            buttons[i] = messageBox.addButton(
+                QString("Button role: %1").arg(i), QMessageBox::ButtonRole(i));
+        }
+
+        return buttons;
+    };
+
+    addAcceptedRow("Accepted_CustomButtons", buttonsCreator);
+    addRejectedRow("Rejected_CustomButtons", buttonsCreator);
+}
+
+static void addStandardButtonsData()
+{
+    ButtonsCreator buttonsCreator = [](QMessageBox &messageBox) {
+        QVector<QPushButton*> buttons;
+        for (int i = QMessageBox::FirstButton; i <= QMessageBox::LastButton; i <<= 1)
+            buttons << messageBox.addButton(QMessageBox::StandardButton(i));
+
+        return buttons;
+    };
+
+    addAcceptedRow("Accepted_StandardButtons", buttonsCreator);
+    addRejectedRow("Rejected_StandardButtons", buttonsCreator);
+}
+
+void tst_QMessageBox::acceptedRejectedSignals_data()
+{
+    QTest::addColumn<RoleSet>("roles");
+    QTest::addColumn<SignalSignature>("signalSignature");
+    QTest::addColumn<ButtonsCreator>("buttonsCreator");
+
+    addStandardButtonsData();
+    addCustomButtonsData();
+}
+
 QTEST_MAIN(tst_QMessageBox)
 #include "tst_qmessagebox.moc"