diff options
author | Christian Ehrlicher <ch.ehrlicher@gmx.de> | 2019-06-10 13:44:42 +0200 |
---|---|---|
committer | Christian Ehrlicher <ch.ehrlicher@gmx.de> | 2019-06-10 13:44:42 +0200 |
commit | a7cbb8c639487edbabc08ea99498503b33c9f6d6 (patch) | |
tree | 5bad01705226bfbc421974feac4682f474ed260b | |
parent | a489e11b97ca6c0f5893711a363c9896dd8377f8 (diff) |
QWidget: fix setTabOrder for compound widgets
81e298a51d08c510457b4a26b37c0d4aac5eba65 fixed a case where the focus
chain was screwed up when the order was already correct. This worked
correctly in most cases but not when the next focus widget of the first
one had Qt::NoFocus.
The optimization check if lastFocusChildOfFirst is the same as second is
thrown away since it now does not longer screw up the focus chain and
the save would only be four pointer assignments.
Fixes: QTBUG-75388
Task-number: QTBUG-10907
Task-number: QTBUG-68393
Task-number: QTBUG-69619
Change-Id: I581ed532156c34ea970123afd063194aab016304
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>
-rw-r--r-- | src/widgets/kernel/qwidget.cpp | 60 | ||||
-rw-r--r-- | tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp | 73 |
2 files changed, 105 insertions, 28 deletions
diff --git a/src/widgets/kernel/qwidget.cpp b/src/widgets/kernel/qwidget.cpp index 8b225df8be..6ef3a4f163 100644 --- a/src/widgets/kernel/qwidget.cpp +++ b/src/widgets/kernel/qwidget.cpp @@ -1,4 +1,4 @@ -/**************************************************************************** +/**************************************************************************** ** ** Copyright (C) 2017 The Qt Company Ltd. ** Copyright (C) 2016 Intel Corporation. @@ -7017,37 +7017,41 @@ void QWidget::setTabOrder(QWidget* first, QWidget *second) lastFocusChild = focusNext; } }; + auto setPrev = [](QWidget *w, QWidget *prev) + { + w->d_func()->focus_prev = prev; + }; + auto setNext = [](QWidget *w, QWidget *next) + { + w->d_func()->focus_next = next; + }; - QWidget *lastFocusChildOfFirst, *lastFocusChildOfSecond; - determineLastFocusChild(first, lastFocusChildOfFirst); + // remove the second widget from the chain + QWidget *lastFocusChildOfSecond; determineLastFocusChild(second, lastFocusChildOfSecond); - - // If the tab order is already correct, exit early - if (lastFocusChildOfFirst == second || - lastFocusChildOfFirst->d_func()->focus_next == second) { - return; + { + QWidget *oldPrev = second->d_func()->focus_prev; + QWidget *prevWithFocus = oldPrev; + while (prevWithFocus->focusPolicy() == Qt::NoFocus) + prevWithFocus = prevWithFocus->d_func()->focus_prev; + // only widgets between first and second -> all is fine + if (prevWithFocus == first) + return; + QWidget *oldNext = lastFocusChildOfSecond->d_func()->focus_next; + setPrev(oldNext, oldPrev); + setNext(oldPrev, oldNext); } - // Note that we need to handle two different sections in the tab chain; The section - // that 'first' belongs to (firstSection), where we are about to insert 'second', and - // the section that 'second' used be a part of (secondSection). When we pull 'second' - // out of the second section and insert it into the first, we also need to ensure - // that we leave the second section in a connected state. - QWidget *firstChainOldSecond = lastFocusChildOfFirst->d_func()->focus_next; - QWidget *secondChainNewFirst = second->d_func()->focus_prev; - QWidget *secondChainNewSecond = lastFocusChildOfSecond->d_func()->focus_next; - - // Insert 'second' after 'first' - lastFocusChildOfFirst->d_func()->focus_next = second; - second->d_func()->focus_prev = lastFocusChildOfFirst; - - // The widget that used to be 'second' in the first section, should now become 'third' - lastFocusChildOfSecond->d_func()->focus_next = firstChainOldSecond; - firstChainOldSecond->d_func()->focus_prev = lastFocusChildOfSecond; - - // Repair the second section after we pulled 'second' out of it - secondChainNewFirst->d_func()->focus_next = secondChainNewSecond; - secondChainNewSecond->d_func()->focus_prev = secondChainNewFirst; + // insert the second widget into the chain + QWidget *lastFocusChildOfFirst; + determineLastFocusChild(first, lastFocusChildOfFirst); + { + QWidget *oldNext = lastFocusChildOfFirst->d_func()->focus_next; + setPrev(second, lastFocusChildOfFirst); + setNext(lastFocusChildOfFirst, second); + setPrev(oldNext, lastFocusChildOfSecond); + setNext(lastFocusChildOfSecond, oldNext); + } } /*!\internal diff --git a/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp b/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp index c0dfaf80a3..489edb703c 100644 --- a/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp +++ b/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp @@ -180,6 +180,7 @@ private slots: void tabOrderWithProxy(); void tabOrderWithCompoundWidgets(); void tabOrderNoChange(); + void tabOrderNoChange2(); #if defined(Q_OS_WIN) && !defined(Q_OS_WINRT) void activation(); #endif @@ -1940,6 +1941,24 @@ static QVector<QWidget*> getFocusChain(QWidget *start, bool bForward) return ret; } +//#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::tabOrderNoChange() { QWidget w; @@ -1951,7 +1970,61 @@ void tst_QWidget::tabOrderNoChange() const auto focusChainForward = getFocusChain(&w, true); const auto focusChainBackward = getFocusChain(&w, false); + dumpFocusChain(&w, true); QWidget::setTabOrder(tabWidget, tv); + dumpFocusChain(&w, true); + QCOMPARE(focusChainForward, getFocusChain(&w, true)); + QCOMPARE(focusChainBackward, getFocusChain(&w, false)); +} + +void tst_QWidget::tabOrderNoChange2() +{ + QWidget w; + auto *verticalLayout = new QVBoxLayout(&w); + auto *tabWidget = new QTabWidget(&w); + tabWidget->setObjectName("tabWidget"); + verticalLayout->addWidget(tabWidget); + + auto *tab1 = new QWidget(tabWidget); + tab1->setObjectName("tab1"); + auto *vLay1 = new QVBoxLayout(tab1); + auto *le1 = new QLineEdit(tab1); + le1->setObjectName("le1"); + auto *le2 = new QLineEdit(tab1); + le2->setObjectName("le2"); + vLay1->addWidget(le1); + vLay1->addWidget(le2); + tabWidget->addTab(tab1, QStringLiteral("Tab 1")); + + auto *tab2 = new QWidget(tabWidget); + tab2->setObjectName("tab2"); + auto *vLay2 = new QVBoxLayout(tab2); + auto *le3 = new QLineEdit(tab2); + le3->setObjectName("le3"); + auto *le4 = new QLineEdit(tab2); + le4->setObjectName("le4"); + vLay2->addWidget(le3); + vLay2->addWidget(le4); + tabWidget->addTab(tab2, QStringLiteral("Tab 2")); + + const auto focusChainForward = getFocusChain(&w, true); + const auto focusChainBackward = getFocusChain(&w, false); + dumpFocusChain(&w, true); + dumpFocusChain(&w, false); + // this will screw up the focus chain order without visible changes, + // so don't call it here for the simplicity of the test + //QWidget::setTabOrder(tabWidget, le1); + + QWidget::setTabOrder(le1, le2); + dumpFocusChain(&w, true, "QWidget::setTabOrder(le1, le2)"); + QWidget::setTabOrder(le2, le3); + dumpFocusChain(&w, true, "QWidget::setTabOrder(le2, le3)"); + QWidget::setTabOrder(le3, le4); + dumpFocusChain(&w, true, "QWidget::setTabOrder(le3, le4)"); + QWidget::setTabOrder(le4, tabWidget); + dumpFocusChain(&w, true, "QWidget::setTabOrder(le4, tabWidget)"); + dumpFocusChain(&w, false); + QCOMPARE(focusChainForward, getFocusChain(&w, true)); QCOMPARE(focusChainBackward, getFocusChain(&w, false)); } |