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

QDialogButtonBox - code cleanup

Browse Source 
Replace const char * based connect statements by modern API and remove
'_q_' slot prefixes.
This requires explicit disconnection in the destructor -> add.
Replace bool traps for layouting and removing buttons by enum classes.
Replace if/elseif event handler by switch.
Replace iterator typedef with auto.
Encapsulate logic to ensure defaulting to first accept button in a
member function for better readability.
Remove dead code.
Move private header declaration from cpp to a new _p.h.

Task-number: QTBUG-114377
Pick-to: 6.6 6.5
Change-Id: I8a2bc355e3816c3c826c10cd96194c89bf0ae510
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
This commit is contained in:
Axel Spoerl 2023-06-24 11:38:31 +02:00
parent 9a320b037c
commit df735d794f
4 changed files with 172 additions and 94 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/widgets/CMakeLists.txt
View File

@ -510,7 +510,7 @@ qt_internal_extend_target(Widgets CONDITION QT_FEATURE_resizehandler
qt_internal_extend_target(Widgets CONDITION QT_FEATURE_dialogbuttonbox
SOURCES
widgets/qdialogbuttonbox.cpp widgets/qdialogbuttonbox.h
widgets/qdialogbuttonbox.cpp widgets/qdialogbuttonbox.h widgets/qdialogbuttonbox_p.h
)
qt_internal_extend_target(Widgets CONDITION QT_FEATURE_rubberband

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

@ -14,6 +14,7 @@
#include <QtGui/qaction.h>
#include "qdialogbuttonbox.h"
#include "qdialogbuttonbox_p.h"
QT_BEGIN_NAMESPACE
@ -112,37 +113,8 @@ QT_BEGIN_NAMESPACE
\sa QMessageBox, QPushButton, QDialog
*/
class QDialogButtonBoxPrivate : public QWidgetPrivate
{
Q_DECLARE_PUBLIC(QDialogButtonBox)
public:
QDialogButtonBoxPrivate(Qt::Orientation orient);
QList<QAbstractButton *> buttonLists[QDialogButtonBox::NRoles];
QHash<QPushButton *, QDialogButtonBox::StandardButton> standardButtonHash;
Qt::Orientation orientation;
QDialogButtonBox::ButtonLayout layoutPolicy;
QBoxLayout *buttonLayout;
bool internalRemove;
bool center;
void createStandardButtons(QDialogButtonBox::StandardButtons buttons);
void layoutButtons();
void initLayout();
void resetLayout();
QPushButton *createButton(QDialogButtonBox::StandardButton button, bool doLayout = true);
void addButton(QAbstractButton *button, QDialogButtonBox::ButtonRole role, bool doLayout = true);
void _q_handleButtonDestroyed();
void _q_handleButtonClicked();
void addButtonsToLayout(const QList<QAbstractButton *> &buttonList, bool reverse);
void retranslateStrings();
};
QDialogButtonBoxPrivate::QDialogButtonBoxPrivate(Qt::Orientation orient)
: orientation(orient), buttonLayout(nullptr), internalRemove(false), center(false)
: orientation(orient), buttonLayout(nullptr), center(false)
{
}
@ -177,7 +149,6 @@ void QDialogButtonBoxPrivate::initLayout()
void QDialogButtonBoxPrivate::resetLayout()
{
//delete buttonLayout;
initLayout();
layoutButtons();
}
@ -311,7 +282,7 @@ void QDialogButtonBoxPrivate::layoutButtons()
}
QPushButton *QDialogButtonBoxPrivate::createButton(QDialogButtonBox::StandardButton sbutton,
bool doLayout)
LayoutRule layoutRule)
{
Q_Q(QDialogButtonBox);
int icon = 0;
@ -386,7 +357,7 @@ QPushButton *QDialogButtonBoxPrivate::createButton(QDialogButtonBox::StandardBut
if (Q_UNLIKELY(role == QPlatformDialogHelper::InvalidRole))
qWarning("QDialogButtonBox::createButton: Invalid ButtonRole, button not added");
else
addButton(button, static_cast<QDialogButtonBox::ButtonRole>(role), doLayout);
addButton(button, static_cast<QDialogButtonBox::ButtonRole>(role), layoutRule);
#if QT_CONFIG(shortcut)
const QKeySequence standardShortcut = QGuiApplicationPrivate::platformTheme()->standardButtonShortcut(sbutton);
if (!standardShortcut.isEmpty())
@ -396,23 +367,26 @@ QPushButton *QDialogButtonBoxPrivate::createButton(QDialogButtonBox::StandardBut
}
void QDialogButtonBoxPrivate::addButton(QAbstractButton *button, QDialogButtonBox::ButtonRole role,
bool doLayout)
LayoutRule layoutRule)
{
Q_Q(QDialogButtonBox);
QObject::connect(button, SIGNAL(clicked()), q, SLOT(_q_handleButtonClicked()));
QObject::connect(button, SIGNAL(destroyed()), q, SLOT(_q_handleButtonDestroyed()));
QObjectPrivate::connect(button, &QAbstractButton::clicked, this, &QDialogButtonBoxPrivate::handleButtonClicked);
QObjectPrivate::connect(button, &QAbstractButton::destroyed, this, &QDialogButtonBoxPrivate::handleButtonDestroyed);
buttonLists[role].append(button);
if (doLayout)
switch (layoutRule) {
case LayoutRule::DoLayout:
layoutButtons();
break;
case LayoutRule::SkipLayout:
break;
}
}
void QDialogButtonBoxPrivate::createStandardButtons(QDialogButtonBox::StandardButtons buttons)
{
uint i = QDialogButtonBox::FirstButton;
while (i <= QDialogButtonBox::LastButton) {
if (i & buttons) {
createButton(QDialogButtonBox::StandardButton(i), false);
}
if (i & buttons)
createButton(QDialogButtonBox::StandardButton(i), LayoutRule::SkipLayout);
i = i << 1;
}
layoutButtons();
@ -420,13 +394,10 @@ void QDialogButtonBoxPrivate::createStandardButtons(QDialogButtonBox::StandardBu
void QDialogButtonBoxPrivate::retranslateStrings()
{
typedef QHash<QPushButton *, QDialogButtonBox::StandardButton>::iterator Iterator;
const Iterator end = standardButtonHash.end();
for (Iterator it = standardButtonHash.begin(); it != end; ++it) {
const QString text = QGuiApplicationPrivate::platformTheme()->standardButtonText(it.value());
for (auto &&[key, value] : std::as_const(standardButtonHash).asKeyValueRange()) {
const QString text = QGuiApplicationPrivate::platformTheme()->standardButtonText(value);
if (!text.isEmpty())
it.key()->setText(text);
key->setText(text);
}
}
@ -482,6 +453,11 @@ QDialogButtonBox::QDialogButtonBox(StandardButtons buttons, Qt::Orientation orie
*/
QDialogButtonBox::~QDialogButtonBox()
{
// 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();
}
/*!
@ -638,7 +614,8 @@ void QDialogButtonBox::clear()
QList<QAbstractButton *> &list = d->buttonLists[i];
while (list.size()) {
QAbstractButton *button = list.takeAt(0);
QObject::disconnect(button, SIGNAL(destroyed()), this, SLOT(_q_handleButtonDestroyed()));
QObjectPrivate::disconnect(button, &QAbstractButton::destroyed,
d, &QDialogButtonBoxPrivate::handleButtonDestroyed);
delete button;
}
}
@ -688,27 +665,30 @@ QDialogButtonBox::ButtonRole QDialogButtonBox::buttonRole(QAbstractButton *butto
void QDialogButtonBox::removeButton(QAbstractButton *button)
{
Q_D(QDialogButtonBox);
d->removeButton(button, QDialogButtonBoxPrivate::RemoveRule::Disconnect);
}
void QDialogButtonBoxPrivate::removeButton(QAbstractButton *button, RemoveRule rule)
{
if (!button)
return;
// Remove it from the standard button hash first and then from the roles
d->standardButtonHash.remove(reinterpret_cast<QPushButton *>(button));
for (int i = 0; i < NRoles; ++i) {
QList<QAbstractButton *> &list = d->buttonLists[i];
for (int j = 0; j < list.size(); ++j) {
if (list.at(j) == button) {
list.takeAt(j);
if (!d->internalRemove) {
disconnect(button, SIGNAL(clicked()), this, SLOT(_q_handleButtonClicked()));
disconnect(button, SIGNAL(destroyed()), this, SLOT(_q_handleButtonDestroyed()));
}
break;
}
}
}
if (!d->internalRemove)
standardButtonHash.remove(reinterpret_cast<QPushButton *>(button));
for (int i = 0; i < QDialogButtonBox::NRoles; ++i)
buttonLists[i].removeOne(button);
switch (rule) {
case RemoveRule::Disconnect:
button->setParent(nullptr);
QObjectPrivate::disconnect(button, &QAbstractButton::clicked,
this, &QDialogButtonBoxPrivate::handleButtonClicked);
QObjectPrivate::disconnect(button, &QAbstractButton::destroyed,
this, &QDialogButtonBoxPrivate::handleButtonDestroyed);
break;
case RemoveRule::KeepConnections:
break;
}
}
/*!
@ -778,7 +758,7 @@ void QDialogButtonBox::setStandardButtons(StandardButtons buttons)
{
Q_D(QDialogButtonBox);
// Clear out all the old standard buttons, then recreate them.
qDeleteAll(d->standardButtonHash.keys());
qDeleteAll(d->standardButtonHash.keyBegin(), d->standardButtonHash.keyEnd());
d->standardButtonHash.clear();
d->createStandardButtons(buttons);
@ -820,7 +800,7 @@ QDialogButtonBox::StandardButton QDialogButtonBox::standardButton(QAbstractButto
return d->standardButtonHash.value(static_cast<QPushButton *>(button));
}
void QDialogButtonBoxPrivate::_q_handleButtonClicked()
void QDialogButtonBoxPrivate::handleButtonClicked()
{
Q_Q(QDialogButtonBox);
if (QAbstractButton *button = qobject_cast<QAbstractButton *>(q->sender())) {
@ -854,13 +834,11 @@ void QDialogButtonBoxPrivate::_q_handleButtonClicked()
}
}
void QDialogButtonBoxPrivate::_q_handleButtonDestroyed()
void QDialogButtonBoxPrivate::handleButtonDestroyed()
{
Q_Q(QDialogButtonBox);
if (QObject *object = q->sender()) {
QBoolBlocker skippy(internalRemove);
q->removeButton(reinterpret_cast<QAbstractButton *>(object));
}
if (QObject *object = q->sender())
removeButton(reinterpret_cast<QAbstractButton *>(object), RemoveRule::KeepConnections);
}
/*!
@ -918,36 +896,66 @@ void QDialogButtonBox::changeEvent(QEvent *event)
}
}
void QDialogButtonBoxPrivate::ensureFirstAcceptIsDefault()
{
Q_Q(QDialogButtonBox);
const QList<QAbstractButton *> &acceptRoleList = buttonLists[QDialogButtonBox::AcceptRole];
QPushButton *firstAcceptButton = acceptRoleList.isEmpty()
? nullptr
: qobject_cast<QPushButton *>(acceptRoleList.at(0));
if (!firstAcceptButton)
return;
bool hasDefault = false;
QWidget *dialog = nullptr;
QWidget *p = q;
while (p && !p->isWindow()) {
p = p->parentWidget();
if ((dialog = qobject_cast<QDialog *>(p)))
break;
}
QWidget *parent = dialog ? dialog : q;
Q_ASSERT(parent);
const auto pushButtons = parent->findChildren<QPushButton *>();
for (QPushButton *pushButton : pushButtons) {
if (pushButton->isDefault() && pushButton != firstAcceptButton) {
hasDefault = true;
break;
}
}
if (!hasDefault && firstAcceptButton)
firstAcceptButton->setDefault(true);
}
void QDialogButtonBoxPrivate::disconnectAll()
{
Q_Q(QDialogButtonBox);
const auto buttons = q->findChildren<QAbstractButton *>();
for (auto *button : buttons)
button->disconnect(q);
}
/*!
\reimp
*/
bool QDialogButtonBox::event(QEvent *event)
{
Q_D(QDialogButtonBox);
if (event->type() == QEvent::Show) {
QList<QAbstractButton *> acceptRoleList = d->buttonLists[AcceptRole];
QPushButton *firstAcceptButton = acceptRoleList.isEmpty() ? 0 : qobject_cast<QPushButton *>(acceptRoleList.at(0));
bool hasDefault = false;
QWidget *dialog = nullptr;
QWidget *p = this;
while (p && !p->isWindow()) {
p = p->parentWidget();
if ((dialog = qobject_cast<QDialog *>(p)))
break;
}
switch (event->type()) {
case QEvent::Show:
d->ensureFirstAcceptIsDefault();
break;
const auto pbs = (dialog ? dialog : this)->findChildren<QPushButton *>();
for (QPushButton *pb : pbs) {
if (pb->isDefault() && pb != firstAcceptButton) {
hasDefault = true;
break;
}
}
if (!hasDefault && firstAcceptButton)
firstAcceptButton->setDefault(true);
}else if (event->type() == QEvent::LanguageChange) {
case QEvent::LanguageChange:
d->retranslateStrings();
break;
default: break;
}
return QWidget::event(event);
}

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

@ -120,8 +120,6 @@ protected:
private:
Q_DISABLE_COPY(QDialogButtonBox)
Q_DECLARE_PRIVATE(QDialogButtonBox)
Q_PRIVATE_SLOT(d_func(), void _q_handleButtonClicked())
Q_PRIVATE_SLOT(d_func(), void _q_handleButtonDestroyed())
};
Q_DECLARE_OPERATORS_FOR_FLAGS(QDialogButtonBox::StandardButtons)

72
src/widgets/widgets/qdialogbuttonbox_p.h Normal file
View File

@ -0,0 +1,72 @@
// Copyright (C) 2023 The Qt Company Ltd.
// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only
#ifndef QDIALOGBUTTONBOX_P_H
#define QDIALOGBUTTONBOX_P_H
//
// W A R N I N G
// -------------
//
// This file is not part of the Qt API. It exists purely as an
// implementation detail. This header file may change from version to
// version without notice, or even be removed.
//
// We mean it.
//
#include <private/qwidget_p.h>
#include <qdialogbuttonbox.h>
QT_BEGIN_NAMESPACE
class Q_AUTOTEST_EXPORT QDialogButtonBoxPrivate : public QWidgetPrivate
{
Q_DECLARE_PUBLIC(QDialogButtonBox)
public:
enum class RemoveRule {
KeepConnections,
Disconnect,
};
enum class LayoutRule {
DoLayout,
SkipLayout,
};
QDialogButtonBoxPrivate(Qt::Orientation orient);
QList<QAbstractButton *> buttonLists[QDialogButtonBox::NRoles];
QHash<QPushButton *, QDialogButtonBox::StandardButton> standardButtonHash;
QHash<QAbstractButton *, QDialogButtonBox::ButtonRole> hiddenButtons;
Qt::Orientation orientation;
QDialogButtonBox::ButtonLayout layoutPolicy;
QBoxLayout *buttonLayout;
bool center;
bool byPassEventFilter = false;
void createStandardButtons(QDialogButtonBox::StandardButtons buttons);
void removeButton(QAbstractButton *button, RemoveRule rule);
void layoutButtons();
void initLayout();
void resetLayout();
QPushButton *createButton(QDialogButtonBox::StandardButton button,
LayoutRule layoutRule = LayoutRule::DoLayout);
void addButton(QAbstractButton *button, QDialogButtonBox::ButtonRole role,
LayoutRule layoutRule = LayoutRule::DoLayout);
void handleButtonDestroyed();
void handleButtonClicked();
bool handleButtonShowAndHide(QAbstractButton *button, QEvent *event);
void addButtonsToLayout(const QList<QAbstractButton *> &buttonList, bool reverse);
void ensureFirstAcceptIsDefault();
void retranslateStrings();
void disconnectAll();
QList<QAbstractButton *> allButtons() const;
QList<QAbstractButton *> visibleButtons() const;
QDialogButtonBox::ButtonRole buttonRole(QAbstractButton *button) const;
};
QT_END_NAMESPACE
#endif // QDIALOGBUTTONBOX_P_H