AuroraMiddleware/qt5base-lts
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

QDialogButtonBox - Update focus chain when buttons show or hide

Browse Source 
Hiding a button in a QDialogButtonBox doesn't remove its default and
focus behavior. Hiding the button shown in the first position, breaks
the focus chain. Tabbing between the button is no longer possible.

This patch implements listening to the buttons' HideToParent and
ShowToParent events. Hidden buttons are removed from the button box
and kept in a separate hash. That ensures focus chain consistency.
When they are shown again, they are added to the button logic and
their default/focus behavior is restored.

An autotest is added in tst_QDialogButtonBox.

Fixes: QTBUG-114377
Pick-to: 6.6 6.5
Change-Id: Id10c4675f43d6007206e41c694688c4f0a34ee52
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
This commit is contained in:
Axel Spoerl 2023-06-24 11:52:18 +02:00
parent ef9fe7a99a
commit bbb71e7e80
5 changed files with 145 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

95
src/widgets/widgets/qdialogbuttonbox.cpp
View File

@ -112,7 +112,6 @@ QT_BEGIN_NAMESPACE
\sa QMessageBox, QPushButton, QDialog
*/
QDialogButtonBoxPrivate::QDialogButtonBoxPrivate(Qt::Orientation orient)
: orientation(orient), buttonLayout(nullptr), center(false)
{
@ -172,6 +171,7 @@ void QDialogButtonBoxPrivate::layoutButtons()
Q_Q(QDialogButtonBox);
const int MacGap = 36 - 8; // 8 is the default gap between a widget and a spacer item
QBoolBlocker blocker(byPassEventFilter);
for (int i = buttonLayout->count() - 1; i >= 0; --i) {
QLayoutItem *item = buttonLayout->takeAt(i);
if (QWidget *widget = item->widget())
@ -367,11 +367,22 @@ QPushButton *QDialogButtonBoxPrivate::createButton(QDialogButtonBox::StandardBut
}
void QDialogButtonBoxPrivate::addButton(QAbstractButton *button, QDialogButtonBox::ButtonRole role,
LayoutRule layoutRule)
LayoutRule layoutRule, AddRule addRule)
{
QObjectPrivate::connect(button, &QAbstractButton::clicked, this, &QDialogButtonBoxPrivate::handleButtonClicked);
QObjectPrivate::connect(button, &QAbstractButton::destroyed, this, &QDialogButtonBoxPrivate::handleButtonDestroyed);
Q_Q(QDialogButtonBox);
buttonLists[role].append(button);
switch (addRule) {
case AddRule::Connect:
QObjectPrivate::connect(button, &QAbstractButton::clicked,
this, &QDialogButtonBoxPrivate::handleButtonClicked);
QObjectPrivate::connect(button, &QAbstractButton::destroyed,
this, &QDialogButtonBoxPrivate::handleButtonDestroyed);
button->installEventFilter(q);
break;
case AddRule::SkipConnect:
break;
}
switch (layoutRule) {
case LayoutRule::DoLayout:
layoutButtons();
@ -453,10 +464,13 @@ QDialogButtonBox::QDialogButtonBox(StandardButtons buttons, Qt::Orientation orie
*/
QDialogButtonBox::~QDialogButtonBox()
{
Q_D(QDialogButtonBox);
d->byPassEventFilter = true;
// QObjectPrivate::connect requires explicit disconnect in destructor
// otherwise the connection may kick in on child destruction and reach
// the parent's destroyed private object
Q_D(QDialogButtonBox);
d->disconnectAll();
}
@ -622,22 +636,32 @@ void QDialogButtonBox::clear()
}
/*!
Returns a list of all the buttons that have been added to the button box.
Returns a list of all buttons that have been added to the button box.
\sa buttonRole(), addButton(), removeButton()
*/
QList<QAbstractButton *> QDialogButtonBox::buttons() const
{
Q_D(const QDialogButtonBox);
return d->allButtons();
}
QList<QAbstractButton *> QDialogButtonBoxPrivate::visibleButtons() const
{
QList<QAbstractButton *> finalList;
for (int i = 0; i < NRoles; ++i) {
const QList<QAbstractButton *> &list = d->buttonLists[i];
for (int i = 0; i < QDialogButtonBox::NRoles; ++i) {
const QList<QAbstractButton *> &list = buttonLists[i];
for (int j = 0; j < list.size(); ++j)
finalList.append(list.at(j));
}
return finalList;
}
QList<QAbstractButton *> QDialogButtonBoxPrivate::allButtons() const
{
return visibleButtons() << hiddenButtons.keys();
}
/*!
Returns the button role for the specified \a button. This function returns
\l InvalidRole if \a button is \nullptr or has not been added to the button box.
@ -654,7 +678,7 @@ QDialogButtonBox::ButtonRole QDialogButtonBox::buttonRole(QAbstractButton *butto
return ButtonRole(i);
}
}
return InvalidRole;
return d->hiddenButtons.value(button, InvalidRole);
}
/*!
@ -670,9 +694,13 @@ void QDialogButtonBox::removeButton(QAbstractButton *button)
void QDialogButtonBoxPrivate::removeButton(QAbstractButton *button, RemoveRule rule)
{
Q_Q(QDialogButtonBox);
if (!button)
return;
// Remove it from hidden buttons
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)
@ -685,6 +713,7 @@ void QDialogButtonBoxPrivate::removeButton(QAbstractButton *button, RemoveRule r
this, &QDialogButtonBoxPrivate::handleButtonClicked);
QObjectPrivate::disconnect(button, &QAbstractButton::destroyed,
this, &QDialogButtonBoxPrivate::handleButtonDestroyed);
button->removeEventFilter(q);
break;
case RemoveRule::KeepConnections:
break;
@ -841,6 +870,54 @@ void QDialogButtonBoxPrivate::handleButtonDestroyed()
removeButton(reinterpret_cast<QAbstractButton *>(object), RemoveRule::KeepConnections);
}
bool QDialogButtonBox::eventFilter(QObject *object, QEvent *event)
{
Q_D(QDialogButtonBox);
if (d->byPassEventFilter)
return false;
QAbstractButton *button = qobject_cast<QAbstractButton *>(object);
if (!button)
return false;
const QEvent::Type type = event->type();
if (type == QEvent::HideToParent || type == QEvent::ShowToParent)
return d->handleButtonShowAndHide(button, event);
return false;
}
bool QDialogButtonBoxPrivate::handleButtonShowAndHide(QAbstractButton *button, QEvent *event)
{
Q_Q(QDialogButtonBox);
const QEvent::Type type = event->type();
switch (type) {
case QEvent::HideToParent: {
const QDialogButtonBox::ButtonRole role = q->buttonRole(button);
if (role != QDialogButtonBox::ButtonRole::InvalidRole) {
removeButton(button, RemoveRule::KeepConnections);
hiddenButtons.insert(button, role);
layoutButtons();
}
break;
}
case QEvent::ShowToParent:
if (hiddenButtons.contains(button)) {
const auto role = hiddenButtons.take(button);
addButton(button, role, LayoutRule::DoLayout, AddRule::SkipConnect);
if (role == QDialogButtonBox::AcceptRole)
ensureFirstAcceptIsDefault();
}
break;
default: break;
}
return false;
}
/*!
\property QDialogButtonBox::centerButtons
\brief whether the buttons in the button box are centered

2
src/widgets/widgets/qdialogbuttonbox.h
View File

@ -107,6 +107,8 @@ public:
void setCenterButtons(bool center);
bool centerButtons() const;
bool eventFilter(QObject *object, QEvent *event) override;
Q_SIGNALS:
void clicked(QAbstractButton *button);
void accepted();

7
src/widgets/widgets/qdialogbuttonbox_p.h
View File

@ -32,6 +32,10 @@ public:
DoLayout,
SkipLayout,
};
enum class AddRule {
Connect,
SkipConnect,
};
QDialogButtonBoxPrivate(Qt::Orientation orient);
@ -54,7 +58,8 @@ public:
QPushButton *createButton(QDialogButtonBox::StandardButton button,
LayoutRule layoutRule = LayoutRule::DoLayout);
void addButton(QAbstractButton *button, QDialogButtonBox::ButtonRole role,
LayoutRule layoutRule = LayoutRule::DoLayout);
LayoutRule layoutRule = LayoutRule::DoLayout,
AddRule addRule = AddRule::Connect);
void handleButtonDestroyed();
void handleButtonClicked();
bool handleButtonShowAndHide(QAbstractButton *button, QEvent *event);

1
tests/auto/widgets/widgets/qdialogbuttonbox/CMakeLists.txt
View File

@ -17,4 +17,5 @@ qt_internal_add_test(tst_qdialogbuttonbox
LIBRARIES
Qt::Gui
Qt::Widgets
Qt::WidgetsPrivate
)

50
tests/auto/widgets/widgets/qdialogbuttonbox/tst_qdialogbuttonbox.cpp
View File

@ -8,6 +8,8 @@
#include <QtWidgets/QDialog>
#include <QtGui/QAction>
#include <qdialogbuttonbox.h>
#include <QtWidgets/private/qdialogbuttonbox_p.h>
#include <QtWidgets/private/qabstractbutton_p.h>
#include <limits.h>
Q_DECLARE_METATYPE(QDialogButtonBox::ButtonRole)
@ -49,6 +51,9 @@ private slots:
void clear();
void removeButton_data();
void removeButton();
#ifdef QT_BUILD_INTERNAL
void hideAndShowButton();
#endif
void buttonRole_data();
void buttonRole();
void setStandardButtons_data();
@ -372,6 +377,51 @@ void tst_QDialogButtonBox::removeButton()
delete button;
}
#ifdef QT_BUILD_INTERNAL
void tst_QDialogButtonBox::hideAndShowButton()
{
QDialogButtonBox buttonBox;
QDialogButtonBoxPrivate *d = static_cast<QDialogButtonBoxPrivate *>(QObjectPrivate::get(&buttonBox));
auto *apply = buttonBox.addButton( "Apply", QDialogButtonBox::ApplyRole );
auto *accept = buttonBox.addButton( "Accept", QDialogButtonBox::AcceptRole );
buttonBox.addButton( "Reject", QDialogButtonBox::RejectRole );
auto *widget = new QWidget();
auto *layout = new QHBoxLayout(widget);
layout->addWidget(&buttonBox);
// apply button shows first on macOS. accept button on all other OSes.
QAbstractButton *hideButton;
#ifdef Q_OS_MACOS
hideButton = apply;
Q_UNUSED(accept);
#else
hideButton = accept;
Q_UNUSED(apply);
#endif
hideButton->hide();
widget->show();
QVERIFY(QTest::qWaitForWindowExposed(widget));
QVERIFY(buttonBox.focusWidget()); // QTBUG-114377: Without fixing, focusWidget() == nullptr
QCOMPARE(d->visibleButtons().count(), 2);
QCOMPARE(d->hiddenButtons.count(), 1);
QVERIFY(d->hiddenButtons.contains(hideButton));
QCOMPARE(buttonBox.buttons().count(), 3);
QSignalSpy spy(qApp, &QApplication::focusChanged);
QTest::sendKeyEvent(QTest::KeyAction::Click, &buttonBox, Qt::Key_Tab, QString(), Qt::KeyboardModifiers());
QCOMPARE(spy.count(), 1); // QTBUG-114377: Without fixing, tabbing wouldn't work.
hideButton->show();
QCOMPARE_GE(spy.count(), 1);
QTRY_COMPARE(QApplication::focusWidget(), hideButton);
QCOMPARE(d->visibleButtons().count(), 3);
QCOMPARE(d->hiddenButtons.count(), 0);
QCOMPARE(buttonBox.buttons().count(), 3);
spy.clear();
QTest::sendKeyEvent(QTest::KeyAction::Click, &buttonBox, Qt::Key_Backtab, QString(), Qt::KeyboardModifiers());
QCOMPARE(spy.count(), 1);
}
#endif
void tst_QDialogButtonBox::testDelete()
{
QDialogButtonBox buttonBox;