QDialogButtonBox: Manage hide and show of standard buttons

0b421fa58b9a73d657bf17834788fd1175c4767e ensured a correct focus chain,
when buttons in a QDialogButtonBox were hidden.

The implementation did not check, if a hidden button was added via
setStandardButtons(). In consequence, it was removed from the
standardButtonHash and never added again.
QDialogButtonBox::button() returned nullptr for a standard button,
once it had been hidden. That introduced a regression.

This follow-up patch makes sure, a standard button is not removed
from standardButtonHash, when hidden. By no longer removing it from
standardButtonHash, it makes showQDialogButtonBox::button() always
return the pointer to the standard button, even if it is hidden.

The function handleButtonDestroyed() used the argument
QDialogButtonBoxPrivate::RemoveRule::KeepConnections, in order to leave
signal/slot connections untouched. It expected the the destroyed button
to be removed from standardButtonHash. In order to retain that
functionality, the enum class RemoveRule is renamed to RemoveReason,
and one value was added. QDialogButtonBoxPrivate now handles all
necessary cases of removing a button:

ManualRemove (previously Disconnect):
- remove button from roles
- remove button from standardButtonHash
- disconnect all signals

LeaveEvent (previously KeepConnections):
- remove button from roles
- do not remove button form standardButtonHash
- do not disconnect signals

Destroyed (new):
- remove button from roles
- remove button from standardButtonHash
- do not disconnect signals (QObject will do that)

An autotest is added to tst_QDialogButtonBox.

Task-number: QTBUG-114377
Change-Id: Ib28625d44fa89c3d06f181f64875c2e456cebbfa
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
(cherry picked from commit edc984db384311e25a0afe4902585bd97e4ebcd1)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit bcdcceca6c4be261e5ca4b600e9275ec84ec7a90)

3 files changed, 44 insertions, 15 deletions

diff --git a/src/widgets/widgets/qdialogbuttonbox.cpp b/src/widgets/widgets/qdialogbuttonbox.cpp
index 69fea04914..c35f8c8edc 100644
--- a/src/widgets/widgets/qdialogbuttonbox.cpp
+++ b/src/widgets/widgets/qdialogbuttonbox.cpp
@@ -704,32 +704,43 @@ QDialogButtonBox::ButtonRole QDialogButtonBox::buttonRole(QAbstractButton *butto
 void QDialogButtonBox::removeButton(QAbstractButton *button)
 {
     Q_D(QDialogButtonBox);
-    d->removeButton(button, QDialogButtonBoxPrivate::RemoveRule::Disconnect);
+    d->removeButton(button, QDialogButtonBoxPrivate::RemoveReason::ManualRemove);
 }
 
-void QDialogButtonBoxPrivate::removeButton(QAbstractButton *button, RemoveRule rule)
+/*!
+   \internal
+   Removes \param button.
+   \param reason determines the behavior following the removal:
+   \list
+   \li \c ManualRemove disconnects all signals and removes the button from standardButtonHash.
+   \li \c HideEvent keeps connections alive, standard buttons remain in standardButtonHash.
+   \li \c Destroyed removes the button from standardButtonHash. Signals remain untouched, because
+          the button might already be only a QObject, the destructor of which handles disconnecting.
+   \endlist
+ */
+void QDialogButtonBoxPrivate::removeButton(QAbstractButton *button, RemoveReason reason)
 {
     if (!button)
         return;
 
-    // Remove it from hidden buttons
+    // Remove button from hidden buttons and roles
     hiddenButtons.remove(button);
-
-    // Remove it from the standard button hash first and then from the roles
-    standardButtonHash.remove(reinterpret_cast<QPushButton *>(button));
     for (int i = 0; i < QDialogButtonBox::NRoles; ++i)
         buttonLists[i].removeOne(button);
 
-    switch (rule) {
-    case RemoveRule::Disconnect:
+    switch (reason) {
+    case RemoveReason::ManualRemove:
         button->setParent(nullptr);
         QObjectPrivate::disconnect(button, &QAbstractButton::clicked,
                                    this, &QDialogButtonBoxPrivate::handleButtonClicked);
         QObjectPrivate::disconnect(button, &QAbstractButton::destroyed,
                                    this, &QDialogButtonBoxPrivate::handleButtonDestroyed);
         button->removeEventFilter(filter.get());
+        Q_FALLTHROUGH();
+    case RemoveReason::Destroyed:
+        standardButtonHash.remove(reinterpret_cast<QPushButton *>(button));
         break;
-    case RemoveRule::KeepConnections:
+    case RemoveReason::HideEvent:
         break;
     }
 }
@@ -881,7 +892,7 @@ void QDialogButtonBoxPrivate::handleButtonDestroyed()
 {
     Q_Q(QDialogButtonBox);
     if (QObject *object = q->sender())
-        removeButton(reinterpret_cast<QAbstractButton *>(object), RemoveRule::KeepConnections);
+        removeButton(reinterpret_cast<QAbstractButton *>(object), RemoveReason::Destroyed);
 }
 
 bool QDialogButtonBoxPrivate::handleButtonShowAndHide(QAbstractButton *button, QEvent *event)
@@ -897,7 +908,7 @@ bool QDialogButtonBoxPrivate::handleButtonShowAndHide(QAbstractButton *button, Q
     case QEvent::HideToParent: {
         const QDialogButtonBox::ButtonRole role = q->buttonRole(button);
         if (role != QDialogButtonBox::ButtonRole::InvalidRole) {
-            removeButton(button, RemoveRule::KeepConnections);
+            removeButton(button, RemoveReason::HideEvent);
             hiddenButtons.insert(button, role);
             layoutButtons();
         }
diff --git a/src/widgets/widgets/qdialogbuttonbox_p.h b/src/widgets/widgets/qdialogbuttonbox_p.h
index 1aa6071241..c3d7e03489 100644
--- a/src/widgets/widgets/qdialogbuttonbox_p.h
+++ b/src/widgets/widgets/qdialogbuttonbox_p.h
@@ -25,9 +25,10 @@ class Q_AUTOTEST_EXPORT QDialogButtonBoxPrivate : public QWidgetPrivate
     Q_DECLARE_PUBLIC(QDialogButtonBox)
 
 public:
-    enum class RemoveRule {
-        KeepConnections,
-        Disconnect,
+    enum class RemoveReason {
+        HideEvent,
+        ManualRemove,
+        Destroyed,
     };
     enum class LayoutRule {
         DoLayout,
@@ -53,7 +54,7 @@ public:
 
     void createStandardButtons(QDialogButtonBox::StandardButtons buttons);
 
-    void removeButton(QAbstractButton *button, RemoveRule rule);
+    void removeButton(QAbstractButton *button, RemoveReason reason);
     void layoutButtons();
     void initLayout();
     void resetLayout();
diff --git a/tests/auto/widgets/widgets/qdialogbuttonbox/tst_qdialogbuttonbox.cpp b/tests/auto/widgets/widgets/qdialogbuttonbox/tst_qdialogbuttonbox.cpp
index b3f26e5dd8..3f5ffa687b 100644
--- a/tests/auto/widgets/widgets/qdialogbuttonbox/tst_qdialogbuttonbox.cpp
+++ b/tests/auto/widgets/widgets/qdialogbuttonbox/tst_qdialogbuttonbox.cpp
@@ -55,6 +55,7 @@ private slots:
 #ifdef QT_BUILD_INTERNAL
     void hideAndShowButton();
 #endif
+    void hideAndShowStandardButton();
     void buttonRole_data();
     void buttonRole();
     void setStandardButtons_data();
@@ -426,6 +427,22 @@ void tst_QDialogButtonBox::hideAndShowButton()
 }
 #endif
 
+void tst_QDialogButtonBox::hideAndShowStandardButton()
+{
+    QDialogButtonBox buttonBox;
+    buttonBox.setStandardButtons(QDialogButtonBox::Ok | QDialogButtonBox::Cancel);
+    buttonBox.show();
+    QVERIFY(QTest::qWaitForWindowExposed(&buttonBox));
+    auto *button = buttonBox.button(QDialogButtonBox::Cancel);
+    QVERIFY(button);
+    button->hide();
+    QVERIFY(QTest::qWaitFor([button](){ return !button->isVisible(); }));
+    QCOMPARE(button, buttonBox.button(QDialogButtonBox::Cancel));
+    button->show();
+    QVERIFY(QTest::qWaitForWindowExposed(button));
+    QCOMPARE(button, buttonBox.button(QDialogButtonBox::Cancel));
+}
+
 void tst_QDialogButtonBox::testDelete()
 {
     QDialogButtonBox buttonBox;