summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--src/widgets/kernel/qwidget.cpp45
-rw-r--r--tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp125
2 files changed, 149 insertions, 21 deletions
diff --git a/src/widgets/kernel/qwidget.cpp b/src/widgets/kernel/qwidget.cpp
index eb7627d641..38957e9b46 100644
--- a/src/widgets/kernel/qwidget.cpp
+++ b/src/widgets/kernel/qwidget.cpp
@@ -6952,16 +6952,16 @@ void QWidget::setTabOrder(QWidget* first, QWidget *second)
return;
}
- auto determineLastFocusChild = [](QWidget *target, QWidget *&lastFocusChild)
+ const auto determineLastFocusChild = [](QWidget *target, QWidget *noFurtherThan)
{
// Since we need to repeat the same logic for both 'first' and 'second', we add a function that
// determines the last focus child for a widget, taking proxies and compound widgets into account.
// If the target is not a compound widget (it doesn't have a focus proxy that points to a child),
// 'lastFocusChild' will be set to the target itself.
- lastFocusChild = target;
+ QWidget *lastFocusChild = target;
QWidget *focusProxy = target->d_func()->deepestFocusProxy();
- if (!focusProxy || !target->isAncestorOf(focusProxy)) {
+ if (!focusProxy) {
// QTBUG-81097: Another case is possible here. We can have a child
// widget, that sets its focusProxy() to the parent (target).
// An example of such widget is a QLineEdit, nested into
@@ -6974,30 +6974,35 @@ void QWidget::setTabOrder(QWidget* first, QWidget *second)
break;
}
}
- return;
- }
-
- lastFocusChild = focusProxy;
-
- for (QWidget *focusNext = lastFocusChild->d_func()->focus_next;
- focusNext != focusProxy && target->isAncestorOf(focusNext) && focusNext->window() == focusProxy->window();
- focusNext = focusNext->d_func()->focus_next) {
- if (focusNext->focusPolicy() != Qt::NoFocus)
- lastFocusChild = focusNext;
+ } else if (target->isAncestorOf(focusProxy)) {
+ lastFocusChild = focusProxy;
+ for (QWidget *focusNext = lastFocusChild->d_func()->focus_next;
+ focusNext != focusProxy && target->isAncestorOf(focusNext) && focusNext->window() == focusProxy->window();
+ focusNext = focusNext->d_func()->focus_next) {
+ if (focusNext == noFurtherThan)
+ break;
+ if (focusNext->focusPolicy() != Qt::NoFocus)
+ lastFocusChild = focusNext;
+ }
}
+ return lastFocusChild;
};
- auto setPrev = [](QWidget *w, QWidget *prev)
- {
+ auto setPrev = [](QWidget *w, QWidget *prev) {
w->d_func()->focus_prev = prev;
};
- auto setNext = [](QWidget *w, QWidget *next)
- {
+ auto setNext = [](QWidget *w, QWidget *next) {
w->d_func()->focus_next = next;
};
+ // detect inflection in case we have compound widgets
+ QWidget *lastFocusChildOfFirst = determineLastFocusChild(first, second);
+ if (lastFocusChildOfFirst == second)
+ lastFocusChildOfFirst = first;
+ QWidget *lastFocusChildOfSecond = determineLastFocusChild(second, first);
+ if (lastFocusChildOfSecond == first)
+ lastFocusChildOfSecond = second;
+
// remove the second widget from the chain
- QWidget *lastFocusChildOfSecond;
- determineLastFocusChild(second, lastFocusChildOfSecond);
{
QWidget *oldPrev = second->d_func()->focus_prev;
QWidget *prevWithFocus = oldPrev;
@@ -7012,8 +7017,6 @@ void QWidget::setTabOrder(QWidget* first, QWidget *second)
}
// insert the second widget into the chain
- QWidget *lastFocusChildOfFirst;
- determineLastFocusChild(first, lastFocusChildOfFirst);
{
QWidget *oldNext = lastFocusChildOfFirst->d_func()->focus_next;
setPrev(second, lastFocusChildOfFirst);
diff --git a/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp b/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp
index 23d915155e..69b385f95b 100644
--- a/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp
+++ b/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp
@@ -164,6 +164,8 @@ private slots:
void tabOrderWithProxyDisabled();
void tabOrderWithProxyOutOfOrder();
void tabOrderWithCompoundWidgets();
+ void tabOrderWithCompoundWidgetsInflection_data();
+ void tabOrderWithCompoundWidgetsInflection();
void tabOrderWithCompoundWidgetsNoFocusPolicy();
void tabOrderNoChange();
void tabOrderNoChange2();
@@ -2271,11 +2273,19 @@ static QList<QWidget *> getFocusChain(QWidget *start, bool bForward)
{
QList<QWidget *> ret;
QWidget *cur = start;
+ // detect infinite loop
+ int count = 100;
+ auto loopGuard = qScopeGuard([]{
+ QFAIL("Inifinite loop detected in focus chain");
+ });
do {
ret += cur;
auto widgetPrivate = static_cast<QWidgetPrivate *>(qt_widget_private(cur));
cur = bForward ? widgetPrivate->focus_next : widgetPrivate->focus_prev;
+ if (!--count)
+ return ret;
} while (cur != start);
+ loopGuard.dismiss();
return ret;
}
@@ -2340,6 +2350,121 @@ void tst_QWidget::tabOrderWithProxyOutOfOrder()
QCOMPARE(QApplication::focusWidget(), &cancelButton);
}
+static bool isFocusChainConsistent(QWidget *widget)
+{
+ auto forward = getFocusChain(widget, true);
+ auto backward = getFocusChain(widget, false);
+ auto logger = qScopeGuard([=]{
+ qCritical("Focus chain is not consistent!");
+ qWarning() << forward.size() << "forwards: " << forward;
+ qWarning() << backward.size() << "backwards:" << backward;
+ });
+ // both lists start with the same, the widget
+ if (forward.takeFirst() != backward.takeFirst())
+ return false;
+ const qsizetype chainLength = forward.size();
+ if (backward.size() != chainLength)
+ return false;
+ for (qsizetype i = 0; i < chainLength; ++i) {
+ if (forward.at(i) != backward.at(chainLength - i - 1))
+ return false;
+ }
+ logger.dismiss();
+ return true;
+}
+
+/*
+ This tests that we end up with consistent and complete chains when we set
+ the tab order from a widget (the lineEdit) inside a compound (the tabWidget)
+ to the compound, or visa versa. In that case, QWidget::setTabOrder will walk
+ the focus chain to the focus child inside the compound to replace the compound
+ itself when manipulating the tab order. If that last focus child is then
+ however also the lineEdit, then we must not create an inconsistent or
+ incomplete loop.
+
+ The tabWidget is seen as a compound because QTabWidget sets the tab bar as
+ the focus proxy, and it has more widgets inside, like pages, toolbuttons etc.
+*/
+void tst_QWidget::tabOrderWithCompoundWidgetsInflection_data()
+{
+ QTest::addColumn<QByteArrayList>("tabOrder");
+
+ QTest::addRow("forward")
+ << QByteArrayList{"dialog", "tabWidget", "lineEdit", "compound", "okButton", "cancelButton"};
+ QTest::addRow("backward")
+ << QByteArrayList{"dialog", "cancelButton", "okButton", "compound", "lineEdit", "tabWidget"};
+}
+
+void tst_QWidget::tabOrderWithCompoundWidgetsInflection()
+{
+ QFETCH(const QByteArrayList, tabOrder);
+
+ QDialog dialog;
+ dialog.setObjectName("dialog");
+ QTabWidget *tabWidget = new QTabWidget;
+ tabWidget->setObjectName("tabWidget");
+ tabWidget->setFocusPolicy(Qt::TabFocus);
+ QWidget *page = new QWidget;
+ page->setObjectName("page");
+ QLineEdit *lineEdit = new QLineEdit;
+ lineEdit->setObjectName("lineEdit");
+ QWidget *compound = new QWidget;
+ compound->setObjectName("compound");
+ compound->setFocusPolicy(Qt::TabFocus);
+ QPushButton *okButton = new QPushButton("Ok");
+ okButton->setObjectName("okButton");
+ okButton->setFocusPolicy(Qt::TabFocus);
+ QPushButton *cancelButton = new QPushButton("Cancel");
+ cancelButton->setObjectName("cancelButton");
+ cancelButton->setFocusPolicy(Qt::TabFocus);
+
+ QVBoxLayout *pageLayout = new QVBoxLayout;
+ pageLayout->addWidget(lineEdit);
+ page->setLayout(pageLayout);
+ tabWidget->addTab(page, "Tab");
+
+ QHBoxLayout *compoundLayout = new QHBoxLayout;
+ compoundLayout->addStretch();
+ compoundLayout->addWidget(cancelButton);
+ compoundLayout->addWidget(okButton);
+ compound->setFocusProxy(okButton);
+ compound->setLayout(compoundLayout);
+
+ QVBoxLayout *dialogLayout = new QVBoxLayout;
+ dialogLayout->addWidget(tabWidget);
+ dialogLayout->addWidget(compound);
+ dialog.setLayout(dialogLayout);
+
+ QVERIFY(isFocusChainConsistent(&dialog));
+
+ QList<QWidget *> expectedFocusChain;
+ for (qsizetype i = 0; i < tabOrder.size() - 1; ++i) {
+ QWidget *first = dialog.findChild<QWidget *>(tabOrder.at(i));
+ if (!first && tabOrder.at(i) == dialog.objectName())
+ first = &dialog;
+ QVERIFY(first);
+ if (i == 0)
+ expectedFocusChain.append(first);
+ QWidget *second = dialog.findChild<QWidget *>(tabOrder.at(i + 1));
+ QVERIFY(second);
+ expectedFocusChain.append(second);
+ QWidget::setTabOrder(first, second);
+ QVERIFY(isFocusChainConsistent(&dialog));
+ }
+
+ const auto forwardChain = getFocusChain(&dialog, true);
+ auto logger = qScopeGuard([=]{
+ qCritical("Order of widgets in focus chain not matching:");
+ qCritical() << " Actual :" << forwardChain;
+ qCritical() << " Expected:" << expectedFocusChain;
+ });
+ for (qsizetype i = 0; i < expectedFocusChain.size() - 2; ++i) {
+ QCOMPARE_LT(forwardChain.indexOf(expectedFocusChain.at(i)),
+ forwardChain.indexOf(expectedFocusChain.at(i + 1)));
+ }
+ logger.dismiss();
+}
+
void tst_QWidget::tabOrderWithCompoundWidgetsNoFocusPolicy()
{
Container container;