Explicitly focus automatic default button in QDialogButtonBox inside dialog

QDialogButtonBox has logic to automatically resolve a default button,
in case one is not already set via QPushButton directly, or via e.g.
QMessageBox::setDefaultButton().

Unfortunately, this default button can in some cases be overridden by
the first button in the dialog button box. The reason this happens
is that the first button is the focus proxy of the button box, so
when the button box gains focus, it will transfer it to the first
button. And since QPushButtons inside a QDialog have their autoDefault
property set to true by default, this focus transfer will also reset
the default button to the focused button.

This arbitrarily happens for any role that happens to be earlier in
the QDialogButtonBox::ButtonLayout than the AcceptRole, and can be
very confusing for the user, so we try to avoid the situation by
explicitly setting the automatic default button as the focus
widget of the button box, unless one is set already.

This is also what QMessageBox::setDefaultButton() does internally.

The only case we're not covering is when the user sets a push button
as default via QPushButton::setDefault(), but we conservatively
assume that the user then also calls setFocus() on the button.

Pick-to: 6.5 6.6
Change-Id: Ibdac0a51ba9439321d4fd7a1dac1ed695e11d693
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>

1 files changed, 49 insertions, 0 deletions

diff --git a/tests/auto/widgets/widgets/qdialogbuttonbox/tst_qdialogbuttonbox.cpp b/tests/auto/widgets/widgets/qdialogbuttonbox/tst_qdialogbuttonbox.cpp
index 3f5ffa687b..9f405a8beb 100644
--- a/tests/auto/widgets/widgets/qdialogbuttonbox/tst_qdialogbuttonbox.cpp
+++ b/tests/auto/widgets/widgets/qdialogbuttonbox/tst_qdialogbuttonbox.cpp
@@ -81,6 +81,7 @@ private slots:
 
     void task191642_default();
     void testDeletedStandardButton();
+    void automaticDefaultButton();
 
 private:
     qint64 timeStamp;
@@ -909,5 +910,53 @@ void tst_QDialogButtonBox::testDeletedStandardButton()
     QVERIFY(!buttonC);
 }
 
+void tst_QDialogButtonBox::automaticDefaultButton()
+{
+    // Having a QDialogButtonBox inside a QDialog triggers Qt to
+    // enable autoDefault for QPushButtons inside the button box.
+    // Check that the logic for resolving a default button based
+    // on the Accept role is not overridden by the first button
+    // in the dialog (the focus proxy) taking focus, and hence
+    // stealing the default button state.
+
+    {
+        QDialog dialog;
+        QDialogButtonBox *bb = new QDialogButtonBox(&dialog);
+        // Force horizontal orientation, where we know the order between
+        // Reset and Accept roles are always the same for all layouts.
+        bb->setOrientation(Qt::Horizontal);
+        auto *okButton = bb->addButton(QDialogButtonBox::Ok);
+        auto *resetButton = bb->addButton(QDialogButtonBox::Reset);
+        // Double check our assumption about Reset being first
+        QCOMPARE(bb->layout()->itemAt(0)->widget(), resetButton);
+
+        dialog.show();
+        QVERIFY(QTest::qWaitForWindowActive(&dialog));
+
+        QVERIFY(okButton->isDefault());
+        QSignalSpy buttonClicked(okButton, &QPushButton::clicked);
+        QTest::keyPress(&dialog, Qt::Key_Enter);
+        QCOMPARE(buttonClicked.count(), 1);
+    }
+
+    // However, if an explicit button has been focused, we respect that.
+
+    {
+        QDialog dialog;
+        QDialogButtonBox *bb = new QDialogButtonBox(&dialog);
+        bb->setOrientation(Qt::Horizontal);
+        bb->addButton(QDialogButtonBox::Ok);
+        auto *resetButton = bb->addButton(QDialogButtonBox::Reset);
+        resetButton->setFocus();
+        dialog.show();
+        QVERIFY(QTest::qWaitForWindowActive(&dialog));
+
+        QVERIFY(resetButton->isDefault());
+        QSignalSpy buttonClicked(resetButton, &QPushButton::clicked);
+        QTest::keyPress(&dialog, Qt::Key_Enter);
+        QCOMPARE(buttonClicked.count(), 1);
+    }
+}
+
 QTEST_MAIN(tst_QDialogButtonBox)
 #include "tst_qdialogbuttonbox.moc"