From b1802a164b8682ed9e8956a5a19a90ade65c25d0 Mon Sep 17 00:00:00 2001 From: Axel Spoerl Date: Wed, 19 Apr 2023 18:46:33 +0200 Subject: Handle parent being a child's focus procy in QWidget::setFocusProxy When a parent became a new child's focus proxy in an existing focus chain, the child was appended at the end of the chain. That leads to broken tab order, e.g. with a QComboBox which became editable after a focus chain has been set. This patch captures the case and insertes the new child after its parent in the focus chain. A corresponding test function is added in tst_QWidget. Fixes: QTBUG-111978 Pick-to: 6.5 Change-Id: I3a426c0560fa830b7b7ffead54f26dd0adef499f Reviewed-by: Volker Hilsheimer --- src/widgets/kernel/qwidget.cpp | 24 ++++++ tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp | 99 +++++++++++++++++++++++ 2 files changed, 123 insertions(+) diff --git a/src/widgets/kernel/qwidget.cpp b/src/widgets/kernel/qwidget.cpp index cde0542eee..d5dbc3f21a 100644 --- a/src/widgets/kernel/qwidget.cpp +++ b/src/widgets/kernel/qwidget.cpp @@ -6412,6 +6412,30 @@ void QWidget::setFocusProxy(QWidget * w) d->focus_prev = oldPrev; oldPrev->d_func()->focus_next = this; firstChild->d_func()->focus_prev = this; + } else if (w->isAncestorOf(this)) { + // If the focus proxy is a parent, 'this' has to be inserted directly after its parent in the focus chain + // remove it from the chain and insert this into the focus chain after its parent + + // is this the case already? + QWidget *parentsNext = w->d_func()->focus_next; + if (parentsNext == this) { + // nothing to do. + Q_ASSERT(d->focus_prev == w); + } else { + // Remove 'this' from the focus chain by making prev and next point directly to each other + QWidget *myOldNext = d->focus_next; + QWidget *myOldPrev = d->focus_prev; + if (myOldNext && myOldPrev) { + myOldNext->d_func()->focus_prev = myOldPrev; + myOldPrev->d_func()->focus_next = myOldNext; + } + + // Insert 'this' behind the parent + w->d_func()->focus_next = this; + d->focus_prev = w; + d->focus_next = parentsNext; + parentsNext->d_func()->focus_prev = this; + } } if (moveFocusToProxy) diff --git a/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp b/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp index 8e497bbb30..7ed993deac 100644 --- a/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp +++ b/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp @@ -49,6 +49,7 @@ #include #include #include +#include #include #include @@ -174,6 +175,8 @@ private slots: void explicitTabOrderWithComplexWidget(); void explicitTabOrderWithSpinBox_QTBUG81097(); void tabOrderList(); + void tabOrderComboBox_data(); + void tabOrderComboBox(); #if defined(Q_OS_WIN) void activation(); #endif @@ -2080,6 +2083,102 @@ void tst_QWidget::tabOrderList() QList({&c, c.lineEdit1, c.lineEdit3, c.lineEdit2})); } +void tst_QWidget::tabOrderComboBox_data() +{ + QTest::addColumn("editableAtBeginning"); + QTest::addColumn>("firstTabOrder"); + QTest::addColumn>("secondTabOrder"); + + QTest::addRow("3 not editable") << false << QList{2, 1, 0} << QList{0, 1, 2}; + QTest::addRow("4 editable") << true << QList{2, 1, 0, 3} << QList{3, 0, 2, 1}; +} + +QWidgetList expectedFocusChain(const QList &boxes, const QList &sequence) +{ + Q_ASSERT(boxes.count() == sequence.count()); + QWidgetList widgets; + for (int i : sequence) { + Q_ASSERT(i >= 0); + Q_ASSERT(i < boxes.count()); + QComboBox *box = boxes.at(i); + widgets.append(box); + if (box->lineEdit()) + widgets.append(box->lineEdit()); + } + + return widgets; +} + +QWidgetList realFocusChain(const QList &boxes, const QList &sequence) +{ + QWidgetList widgets = getFocusChain(boxes.at(sequence.at(0)), true); + // Filter everything with NoFocus + for (auto *widget : widgets) { + if (widget->focusPolicy() == Qt::NoFocus) + widgets.removeOne(widget); + } + return widgets; +} + +void setTabOrder(const QList &boxes, const QList &sequence) +{ + Q_ASSERT(boxes.count() == sequence.count()); + QWidget *previous = nullptr; + for (int i : sequence) { + Q_ASSERT(i >= 0); + Q_ASSERT(i < boxes.count()); + QWidget *box = boxes.at(i); + if (!previous) { + previous = box; + } else { + QWidget::setTabOrder(previous, box); + previous = box; + } + } +} + +void tst_QWidget::tabOrderComboBox() +{ + QFETCH(const bool, editableAtBeginning); + QFETCH(const QList, firstTabOrder); + QFETCH(const QList, secondTabOrder); + const int count = firstTabOrder.count(); + Q_ASSERT(count == secondTabOrder.count()); + + QWidget w; + w.setObjectName("MainWidget"); + QVBoxLayout* layout = new QVBoxLayout(); + w.setLayout(layout); + + QList boxes; + for (int i = 0; i < count; ++i) { + auto box = new QComboBox; + box->setObjectName("ComboBox " + QString::number(i)); + if (editableAtBeginning) { + box->setEditable(true); + box->lineEdit()->setObjectName("LineEdit " + QString::number(i)); + } + boxes.append(box); + layout->addWidget(box); + } + layout->addStretch(); + +#define COMPARE(seq)\ + setTabOrder(boxes, seq);\ + QCOMPARE(realFocusChain(boxes, seq), expectedFocusChain(boxes, seq)) + + COMPARE(firstTabOrder); + + if (!editableAtBeginning) { + for (auto *box : boxes) + box->setEditable(box); + } + + COMPARE(secondTabOrder); + +#undef COMPARE +} + void tst_QWidget::tabOrderWithProxy() { if (QGuiApplication::platformName().startsWith(QLatin1String("wayland"), Qt::CaseInsensitive)) -- cgit v1.2.3