whytofly
/
qt6-bb10
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Handle tab order inception 

If a composite widget is put behind one of it's contained children via
QWidget::setTabOrder, then our logic might replace the composite widget
with the contained child. In that case, we'd end up with a broken tab
chain, possibly resulting in incomplete clean-ups and triggering asserts
when shutting down the UI.

Handle this by stopping the last-child searching logic at the respective
other widget, and by not allowing both widgets to be the same.

Augment test case, and apply some minor refactoring to the code.

Pick-to: 6.5 6.2
Change-Id: I7d97dfa85315b6c01daa283253025d94a1727048
Reviewed-by: Axel Spoerl <axel.spoerl@qt.io>
bb10
Volker Hilsheimer 2023-03-06 12:05:30 +01:00
parent 06f60b24e2
commit 9255820ad5
2 changed files with 149 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
src/widgets/kernel/qwidget.cpp
View File

@ -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);

125
tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp
View File

@ -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;