1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix focus chain with compound widgets if created out of order

Browse Source 
When a compound widget is created not directly before its children,
then another widget will be in the focus chain between the compound and
the compound's first child. If one of those children is then made the
focus proxy of the compound, then the widget in between becomes
unreachable by tabbing.

To fix this, detect that we set the focus proxy to be a descendent of
the compound widget, and then move the compound widget directly in front
of its first child in the focus chain. This way we can't have any gaps
in the focus chain.

Augment the test case with a corresponding scenario. As a drive-by, move
the debug helper up in the code so that it can be easier used, and set
object names on relevant widgets.

Fixes: QTBUG-89156
Change-Id: I17057719a90f59629087afbd1d2ca58c1aa1d8f6
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>
(cherry picked from commit 46648436d4dae529b81f5d7a987c0016cf54bf6d)
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
This commit is contained in:
Volker Hilsheimer 2022-10-27 19:36:55 +02:00
parent a009aa97b1
commit 28324c0a0b
2 changed files with 109 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -6381,6 +6381,33 @@ void QWidget::setFocusProxy(QWidget * w)
d->createExtra();
d->extra->focus_proxy = w;
if (w && isAncestorOf(w)) {
// If the focus proxy is a child of this (so this is a compound widget), then
// we need to make sure that this widget is immediately in front of its own children
// in the focus chain. Otherwise focusNextPrev_helper might jump over unrelated
// widgets that are positioned between this compound widget, and its proxy in
// the focus chain.
const QWidget *parentOfW = w->parentWidget();
Q_ASSERT(parentOfW); // can't be nullptr since we are an ancestor of w
QWidget *firstChild = nullptr;
const auto childList = children();
for (QObject *child : childList) {
if ((firstChild = qobject_cast<QWidget *>(child)))
break;
}
Q_ASSERT(firstChild); // can't be nullptr since w is a child
QWidget *oldNext = d->focus_next;
QWidget *oldPrev = d->focus_prev;
oldNext->d_func()->focus_prev = oldPrev;
oldPrev->d_func()->focus_next = oldNext;
oldPrev = firstChild->d_func()->focus_prev;
d->focus_next = firstChild;
d->focus_prev = oldPrev;
oldPrev->d_func()->focus_next = this;
firstChild->d_func()->focus_prev = this;
}
if (moveFocusToProxy)
setFocus(Qt::OtherFocusReason);
}
@ -7072,8 +7099,8 @@ void QWidgetPrivate::reparentFocusWidgets(QWidget * oldtlw)
n->d_func()->focus_next = topLevel;
} else {
//repair the new list
n->d_func()->focus_next = q;
focus_prev = n;
n->d_func()->focus_next = q;
focus_prev = n;
}
}

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

@ -166,6 +166,7 @@ private slots:
void reverseTabOrder();
void tabOrderWithProxy();
void tabOrderWithProxyDisabled();
void tabOrderWithProxyOutOfOrder();
void tabOrderWithCompoundWidgets();
void tabOrderWithCompoundWidgetsNoFocusPolicy();
void tabOrderNoChange();
@ -1899,8 +1900,11 @@ public:
setObjectName(name);
lineEdit1 = new QLineEdit;
lineEdit1->setObjectName(name + "/lineEdit1");
lineEdit2 = new QLineEdit;
lineEdit2->setObjectName(name + "/lineEdit2");
lineEdit3 = new QLineEdit;
lineEdit3->setObjectName(name + "/lineEdit3");
lineEdit3->setEnabled(false);
QHBoxLayout* hbox = new QHBoxLayout(this);
@ -2146,6 +2150,24 @@ void tst_QWidget::tabOrderWithProxyDisabled()
qPrintable(QApplication::focusWidget()->objectName()));
}
//#define DEBUG_FOCUS_CHAIN
static void dumpFocusChain(QWidget *start, bool bForward, const char *desc = nullptr)
{
#ifdef DEBUG_FOCUS_CHAIN
qDebug() << "Dump focus chain, start:" << start << "isForward:" << bForward << desc;
QWidget *cur = start;
do {
qDebug() << "-" << cur;
auto widgetPrivate = static_cast<QWidgetPrivate *>(qt_widget_private(cur));
cur = bForward ? widgetPrivate->focus_next : widgetPrivate->focus_prev;
} while (cur != start);
#else
Q_UNUSED(start);
Q_UNUSED(bForward);
Q_UNUSED(desc);
#endif
}
void tst_QWidget::tabOrderWithCompoundWidgets()
{
if (QGuiApplication::platformName().startsWith(QLatin1String("wayland"), Qt::CaseInsensitive))
@ -2249,22 +2271,65 @@ static QList<QWidget *> getFocusChain(QWidget *start, bool bForward)
return ret;
}
//#define DEBUG_FOCUS_CHAIN
static void dumpFocusChain(QWidget *start, bool bForward, const char *desc = nullptr)
void tst_QWidget::tabOrderWithProxyOutOfOrder()
{
#ifdef DEBUG_FOCUS_CHAIN
qDebug() << "Dump focus chain, start:" << start << "isForward:" << bForward << desc;
QWidget *cur = start;
do {
qDebug() << cur;
auto widgetPrivate = static_cast<QWidgetPrivate *>(qt_widget_private(cur));
cur = bForward ? widgetPrivate->focus_next : widgetPrivate->focus_prev;
} while (cur != start);
#else
Q_UNUSED(start);
Q_UNUSED(bForward);
Q_UNUSED(desc);
#endif
Container container;
container.setWindowTitle(QLatin1String(QTest::currentTestFunction()));
// important to create the widgets with parent so that they are
// added to the focus chain already now, and with the buttonBox
// before the outsideButton.
QWidget buttonBox(&container);
buttonBox.setObjectName("buttonBox");
QPushButton outsideButton(&container);
outsideButton.setObjectName("outsideButton");
container.box->addWidget(&outsideButton);
container.box->addWidget(&buttonBox);
QCOMPARE(getFocusChain(&container, true),
QList<QWidget*>({&container, &buttonBox, &outsideButton}));
// this now adds okButon and cancelButton to the focus chain,
// after the outsideButton - so the outsideButton is in between
// the buttonBox and the children of the buttonBox!
QPushButton okButton(&buttonBox);
okButton.setObjectName("okButton");
QPushButton cancelButton(&buttonBox);
cancelButton.setObjectName("cancelButton");
QCOMPARE(getFocusChain(&container, true),
QList<QWidget*>({&container, &buttonBox, &outsideButton, &okButton, &cancelButton}));
// by setting the okButton as the focusProxy, the outsideButton becomes
// unreachable when navigating the focus chain as the buttonBox is in front
// of, and proxies to the okButton behind the outsideButton. setFocusProxy
// must fix that by moving the buttonBox in front of the first sibling of
// the proxy.
buttonBox.setFocusProxy(&okButton);
QCOMPARE(getFocusChain(&container, true),
QList<QWidget*>({&container, &outsideButton, &buttonBox, &okButton, &cancelButton}));
container.show();
container.activateWindow();
QApplication::setActiveWindow(&container);
if (!QTest::qWaitForWindowActive(&container))
QSKIP("Window failed to activate, skipping test");
QCOMPARE(QApplication::focusWidget(), &outsideButton);
container.tab();
QCOMPARE(QApplication::focusWidget(), &okButton);
container.tab();
QCOMPARE(QApplication::focusWidget(), &cancelButton);
container.tab();
QCOMPARE(QApplication::focusWidget(), &outsideButton);
container.backTab();
QCOMPARE(QApplication::focusWidget(), &cancelButton);
container.backTab();
QCOMPARE(QApplication::focusWidget(), &okButton);
container.backTab();
QCOMPARE(QApplication::focusWidget(), &outsideButton);
container.backTab();
QCOMPARE(QApplication::focusWidget(), &cancelButton);
}
void tst_QWidget::tabOrderWithCompoundWidgetsNoFocusPolicy()