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
Pick-to: 6.6 6.5
Change-Id: Ib28625d44fa89c3d06f181f64875c2e456cebbfa
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>

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) {
+    switch (reason) {
-    case RemoveRule::Disconnect:
+    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();
         }

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 {
+    enum class RemoveReason {
-        KeepConnections,
+        HideEvent,
-        Disconnect,
+        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();

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;