From 1b23260624a565efaef7830d553209175717d9e2 Mon Sep 17 00:00:00 2001 From: Axel Spoerl Date: Thu, 25 Jan 2024 17:07:34 +0100 Subject: [PATCH] QDialogButtonBox: Don't set focus in a dialog with StrongFocus children A QDialogButtonBox with the first accept button becoming default, didn't explicitly set focus on such a button in a QDialog. d44413d526ec12ed83acd7343c2005782178c7ad implemented this missing functionality. It set focus to the automatic default button, unless the QDialog had a focusWidget() set. That has caused a regression, in cases where - the QDialog has a QWidget child with a Qt::StrongFocus policy, and - the QDialog is not yet visible, so focusWidget() returns nullptr. Amend d44413d526ec12ed83acd7343c2005782178c7ad: Implement a helper in QWidgetPrivate, that returns true, if a child with a given focus policy is found. Do not set focus to a QDialogButtonBox's automatic default button, when - not located inside a QDialog, or - a focusWidget() exists, or - the dialog has QWidget child with Qt::StrongFocus, that is not a child of the QDialogButtonBox. Add an autotest function. Fixes: QTBUG-121514 Change-Id: I3c65ae36b56657f9af4a3a4b42f9b66e8bc5c534 Reviewed-by: Jani Heikkinen --- src/widgets/kernel/qwidget.cpp | 18 +++++++++ src/widgets/kernel/qwidget_p.h | 1 + src/widgets/widgets/qdialogbuttonbox.cpp | 6 ++- .../qdialogbuttonbox/tst_qdialogbuttonbox.cpp | 37 +++++++++++++++++++ 4 files changed, 60 insertions(+), 2 deletions(-) diff --git a/src/widgets/kernel/qwidget.cpp b/src/widgets/kernel/qwidget.cpp index 8d1f9f263c4..5c23a779bc6 100644 --- a/src/widgets/kernel/qwidget.cpp +++ b/src/widgets/kernel/qwidget.cpp @@ -13156,6 +13156,24 @@ void QWidgetPrivate::setNetWmWindowTypes(bool skipIfMissing) #endif } +/*! + \internal + \return \c true, if a child with \param policy exists and isn't a child of \param excludeChildrenOf. + Return false otherwise. + */ +bool QWidgetPrivate::hasChildWithFocusPolicy(Qt::FocusPolicy policy, const QWidget *excludeChildrenOf) const +{ + Q_Q(const QWidget); + const QWidgetList &children = q->findChildren(Qt::FindChildrenRecursively); + for (const auto *child : children) { + if (child->focusPolicy() == policy && child->isEnabled() + && (!excludeChildrenOf || !excludeChildrenOf->isAncestorOf(child))) { + return true; + } + } + return false; +} + #ifndef QT_NO_DEBUG_STREAM static inline void formatWidgetAttributes(QDebug debug, const QWidget *widget) diff --git a/src/widgets/kernel/qwidget_p.h b/src/widgets/kernel/qwidget_p.h index 8a86b069a0e..ec9def64094 100644 --- a/src/widgets/kernel/qwidget_p.h +++ b/src/widgets/kernel/qwidget_p.h @@ -737,6 +737,7 @@ public: bool stealKeyboardGrab(bool grab); bool stealMouseGrab(bool grab); + bool hasChildWithFocusPolicy(Qt::FocusPolicy policy, const QWidget *excludeChildrenOf = nullptr) const; }; Q_DECLARE_OPERATORS_FOR_FLAGS(QWidgetPrivate::DrawWidgetFlags) diff --git a/src/widgets/widgets/qdialogbuttonbox.cpp b/src/widgets/widgets/qdialogbuttonbox.cpp index 91d846d9a0b..ca699e3ad46 100644 --- a/src/widgets/widgets/qdialogbuttonbox.cpp +++ b/src/widgets/widgets/qdialogbuttonbox.cpp @@ -1023,8 +1023,10 @@ void QDialogButtonBoxPrivate::ensureFirstAcceptIsDefault() // focus proxy/first button stealing the default button status // immediately when the button box is focused, which is not what // we want. Account for this by explicitly making the firstAcceptButton - // focused as well, unless an explicit focus widget has been set. - if (dialog && !dialog->focusWidget()) + // focused as well, unless an explicit focus widget has been set, or + // a dialog child has Qt::StrongFocus. + if (dialog && !(QWidgetPrivate::get(dialog)->hasChildWithFocusPolicy(Qt::StrongFocus, q) + || dialog->focusWidget())) firstAcceptButton->setFocus(); } } diff --git a/tests/auto/widgets/widgets/qdialogbuttonbox/tst_qdialogbuttonbox.cpp b/tests/auto/widgets/widgets/qdialogbuttonbox/tst_qdialogbuttonbox.cpp index 9f405a8bebe..83abf0e8e36 100644 --- a/tests/auto/widgets/widgets/qdialogbuttonbox/tst_qdialogbuttonbox.cpp +++ b/tests/auto/widgets/widgets/qdialogbuttonbox/tst_qdialogbuttonbox.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -82,6 +83,8 @@ private slots: void task191642_default(); void testDeletedStandardButton(); void automaticDefaultButton(); + void initialFocus_data(); + void initialFocus(); private: qint64 timeStamp; @@ -958,5 +961,39 @@ void tst_QDialogButtonBox::automaticDefaultButton() } } +void tst_QDialogButtonBox::initialFocus_data() +{ + QTest::addColumn("focusPolicy"); + QTest::addColumn("lineEditHasFocus"); + + QTest::addRow("TabFocus") << Qt::FocusPolicy::TabFocus << false; + QTest::addRow("StrongFocus") << Qt::FocusPolicy::StrongFocus << true; + QTest::addRow("NoFocus") << Qt::FocusPolicy::NoFocus << false; + QTest::addRow("ClickFocus") << Qt::FocusPolicy::ClickFocus << false; + QTest::addRow("WheelFocus") << Qt::FocusPolicy::WheelFocus << false; +} + +void tst_QDialogButtonBox::initialFocus() +{ + QFETCH(const Qt::FocusPolicy, focusPolicy); + QFETCH(const bool, lineEditHasFocus); + QDialog dialog; + QVBoxLayout *layout = new QVBoxLayout(&dialog); + QLineEdit *lineEdit = new QLineEdit(&dialog); + lineEdit->setFocusPolicy(focusPolicy); + layout->addWidget(lineEdit); + QDialogButtonBox *dialogButtonBox = new QDialogButtonBox(&dialog); + layout->addWidget(dialogButtonBox); + dialogButtonBox->addButton(QDialogButtonBox::Reset); + const auto *firstAcceptButton = dialogButtonBox->addButton(QDialogButtonBox::Ok); + dialogButtonBox->addButton(QDialogButtonBox::Cancel); + dialog.show(); + dialog.activateWindow(); + if (lineEditHasFocus) + QTRY_VERIFY(lineEdit->hasFocus()); + else + QTRY_VERIFY(firstAcceptButton->hasFocus()); +} + QTEST_MAIN(tst_QDialogButtonBox) #include "tst_qdialogbuttonbox.moc"