diff options
| author | Axel Spoerl <axel.spoerl@qt.io> | 2024-05-10 21:04:57 +0200 |
|---|---|---|
| committer | Axel Spoerl <axel.spoerl@qt.io> | 2024-05-23 01:12:32 +0200 |
| commit | 2d4993899f0dfda1288e891bc4d9a890af851a4f (patch) | |
| tree | 02f4ca2d6fb4fdb29f8e76b70c9a67096bd0b449 /src/widgets/kernel/qwidget.cpp | |
| parent | 709e9d90ab9f7ec962d23d83467a06532f5f771b (diff) | |
Widgets focus abstraction: Fix re-parenting
Focus abstraction in 58d5d4b7c2faaeaa2c2ccdabb3da6d37f9db880a was
supposed to be behavior-neutral. QWidgetPrivate::reparentFocusChildren
used QObject::findChildren() to find children inside and outside the
current focus chain. If the re-parented widget had only direct children
and the focus chain was equal to creation-order, the result was
identical to previous behavior.
When the re-parented widget had grandchildren, the behavior differred.
While not being detected by existing tests, it caused a regression.
Re-implement the previous logic: Iterate through the focus chain,
instead of relying on QObject::findChildren() returning a complete and
well-ordered list.
Modify tst_QWidget::focusChainOnReparent() in order to cover the
regression.
This amends 58d5d4b7c2faaeaa2c2ccdabb3da6d37f9db880a.
Fixes: QTBUG-125257
Change-Id: Iff4f1d0d9b6117c50c8980dfb6eebfc6f6d4a710
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
Diffstat (limited to 'src/widgets/kernel/qwidget.cpp')
| -rw-r--r-- | src/widgets/kernel/qwidget.cpp | 80 |
1 files changed, 54 insertions, 26 deletions
diff --git a/src/widgets/kernel/qwidget.cpp b/src/widgets/kernel/qwidget.cpp index 750b2946766..dfc69c7d93b 100644 --- a/src/widgets/kernel/qwidget.cpp +++ b/src/widgets/kernel/qwidget.cpp @@ -7093,7 +7093,6 @@ void QWidgetPrivate::reparentFocusWidgets(QWidget * oldtlw) if (focus_child) focus_child->clearFocus(); - insertIntoFocusChain(QWidgetPrivate::FocusDirection::Previous, q->window()); reparentFocusChildren(QWidgetPrivate::FocusDirection::Next); } @@ -13369,32 +13368,61 @@ void QWidgetPrivate::initFocusChain() void QWidgetPrivate::reparentFocusChildren(FocusDirection direction) { Q_Q(QWidget); - QWidgetList focusChildrenInsideChain; - QDuplicateTracker<QWidget *> seen; - QWidget *widget = q->nextInFocusChain(); - while (q->isAncestorOf(widget) - && !seen.hasSeen(widget) - && widget != q->window()) { - if (widget->focusPolicy() != Qt::NoFocus) - focusChildrenInsideChain << widget; - - widget = direction == FocusDirection::Next ? widget->nextInFocusChain() - : widget->previousInFocusChain(); - } - - const QWidgetList children = q->findChildren<QWidget *>(Qt::FindDirectChildrenOnly); - QWidgetList focusChildrenOutsideChain; - for (auto *child : children) { - if (!focusChildrenInsideChain.contains(child)) - focusChildrenOutsideChain << child; - } - if (focusChildrenOutsideChain.isEmpty()) - return; - QWidget *previous = q; - for (auto *child : focusChildrenOutsideChain) { - child->d_func()->insertIntoFocusChain(direction, previous); - previous = child; + // separate the focus chain into new (children of myself) and old (the rest) + QWidget *firstOld = nullptr; + QWidget *lastOld = nullptr; // last in the old list + QWidget *lastNew = q; // last in the new list + bool prevWasNew = true; + QWidget *widget = nextPrevElementInFocusChain(direction); + + // For efficiency, do not maintain the list invariant inside the loop. + // Append items to the relevant list, and we optimize by not changing pointers, + // when subsequent items are going into the same list. + while (widget != q) { + bool currentIsNew = q->isAncestorOf(widget); + if (currentIsNew) { + if (!prevWasNew) { + // previous was old => append to new list + FOCUS_NEXT(lastNew) = widget; + FOCUS_PREV(widget) = lastNew; + } + lastNew = widget; + } else { + if (prevWasNew) { + // prev was new => append to old list, if it exists + if (lastOld) { + FOCUS_NEXT(lastOld) = widget; + FOCUS_PREV(widget) = lastOld; + } else { + // start the old list + firstOld = widget; + } + } + lastOld = widget; + } + widget = widget->d_func()->nextPrevElementInFocusChain(direction); + prevWasNew = currentIsNew; + } + + // repair old list: + if (firstOld) { + FOCUS_NEXT(lastOld) = firstOld; + FOCUS_PREV(firstOld) = lastOld; + } + + if (!q->isWindow()) { + QWidget *topLevel = q->window(); + // insert new chain into toplevel's chain + QWidget *prev = FOCUS_PREV(topLevel); + FOCUS_PREV(topLevel) = lastNew; + FOCUS_NEXT(prev) = q; + FOCUS_PREV(q) = prev; + FOCUS_NEXT(lastNew) = topLevel; + } else { + // repair new list + FOCUS_NEXT(lastNew) = q; + FOCUS_PREV(q) = lastNew; } } |