From 78a3301372fb9b48dc65b18a19731db37abab75c 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 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Pick-to: 6.7 6.6 6.5 Fixes: QTBUG-121514 Fixes: QTBUG-120049 Change-Id: I3c65ae36b56657f9af4a3a4b42f9b66e8bc5c534 Reviewed-by: Tor Arne Vestbø --- 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 81c6a6f7d83..ec7b5ce69ae 100644 --- a/src/widgets/kernel/qwidget.cpp +++ b/src/widgets/kernel/qwidget.cpp @@ -13235,6 +13235,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 namespace { diff --git a/src/widgets/kernel/qwidget_p.h b/src/widgets/kernel/qwidget_p.h index b677a7eccb5..e2e39630de5 100644 --- a/src/widgets/kernel/qwidget_p.h +++ b/src/widgets/kernel/qwidget_p.h @@ -741,6 +741,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 545e8bb6dab..30ace89fa86 100644 --- a/src/widgets/widgets/qdialogbuttonbox.cpp +++ b/src/widgets/widgets/qdialogbuttonbox.cpp @@ -1029,8 +1029,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 65098f2fde6..3e7dc92da11 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"