diff options
author | Volker Hilsheimer <volker.hilsheimer@qt.io> | 2021-07-22 22:32:41 +0200 |
---|---|---|
committer | Volker Hilsheimer <volker.hilsheimer@qt.io> | 2021-12-06 09:09:21 +0100 |
commit | fae2460e925fd7baab85cf567dd9c6093647f4b9 (patch) | |
tree | f21bf9a437608017de6a306c1dfe4541cbcadf55 | |
parent | f3c5fc5a01e888b6ae6a8002221c86e722b70f99 (diff) |
Fix setting the focus reason in Qt Quick Controls
Focus might go to the content item, which is a QQuickItem that has no
concept of either focusPolicy or focusReason. For Qt Quick Controls to
update the focusReason property when an item in it's item tree receives
or loses focus, it has to install itself as a notification listener on
the item.
However, QQuickItemChangeListener didn't have any notification type for
focus changes.
Add the new change type, and notify listeners whenever the an item's
focus changes. Focus scoping might mean that an item never loses focus,
but nevertheless loses active focus.
To be able to update items recursively with the correct focusReason, add
the reason parameter to the QQuickDeliveryAgent helper function.
Fixes: QTBUG-75862
Change-Id: I3d19b722bb07b55416b8cfbb4a9cdb0edd0da3ec
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>
(cherry picked from commit 61ee61d8c977ecf876ce1d364cff634467b562a9)
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
-rw-r--r-- | src/quick/items/qquickitem.cpp | 9 | ||||
-rw-r--r-- | src/quick/items/qquickitem_p.h | 1 | ||||
-rw-r--r-- | src/quick/util/qquickdeliveryagent.cpp | 23 | ||||
-rw-r--r-- | src/quick/util/qquickdeliveryagent_p_p.h | 2 | ||||
-rw-r--r-- | src/quicktemplates2/qquickcontrol.cpp | 15 | ||||
-rw-r--r-- | src/quicktemplates2/qquickcontrol_p_p.h | 1 | ||||
-rw-r--r-- | tests/auto/quickcontrols2/focus/tst_focus.cpp | 10 |
7 files changed, 41 insertions, 20 deletions
diff --git a/src/quick/items/qquickitem.cpp b/src/quick/items/qquickitem.cpp index b1b5432b89..09eccb289c 100644 --- a/src/quick/items/qquickitem.cpp +++ b/src/quick/items/qquickitem.cpp @@ -7385,6 +7385,7 @@ void QQuickItem::setFocus(bool focus, Qt::FocusReason reason) if (d->focus == focus) return; + bool notifyListeners = false; if (d->window || d->parentItem) { // Need to find our nearest focus scope QQuickItem *scope = parentItem(); @@ -7416,9 +7417,10 @@ void QQuickItem::setFocus(bool focus, Qt::FocusReason reason) d->focus = focus; changed << this; + notifyListeners = true; emit focusChanged(focus); - QQuickDeliveryAgentPrivate::notifyFocusChangesRecur(changed.data(), changed.count() - 1); + QQuickDeliveryAgentPrivate::notifyFocusChangesRecur(changed.data(), changed.count() - 1, reason); } } else { QVarLengthArray<QQuickItem *, 20> changed; @@ -7431,10 +7433,13 @@ void QQuickItem::setFocus(bool focus, Qt::FocusReason reason) d->focus = focus; changed << this; + notifyListeners = true; emit focusChanged(focus); - QQuickDeliveryAgentPrivate::notifyFocusChangesRecur(changed.data(), changed.count() - 1); + QQuickDeliveryAgentPrivate::notifyFocusChangesRecur(changed.data(), changed.count() - 1, reason); } + if (notifyListeners) + d->notifyChangeListeners(QQuickItemPrivate::Focus, &QQuickItemChangeListener::itemFocusChanged, this, reason); } /*! diff --git a/src/quick/items/qquickitem_p.h b/src/quick/items/qquickitem_p.h index c7f9cc05ed..4406e191cb 100644 --- a/src/quick/items/qquickitem_p.h +++ b/src/quick/items/qquickitem_p.h @@ -331,6 +331,7 @@ public: ImplicitWidth = 0x100, ImplicitHeight = 0x200, Enabled = 0x400, + Focus = 0x800, AllChanges = 0xFFFFFFFF }; diff --git a/src/quick/util/qquickdeliveryagent.cpp b/src/quick/util/qquickdeliveryagent.cpp index f613e74d53..96a6fee35a 100644 --- a/src/quick/util/qquickdeliveryagent.cpp +++ b/src/quick/util/qquickdeliveryagent.cpp @@ -399,7 +399,9 @@ void QQuickDeliveryAgentPrivate::setFocusInScope(QQuickItem *scope, QQuickItem * if (item != rootItem && !(options & DontChangeSubFocusItem)) { QQuickItem *oldSubFocusItem = scopePrivate->subFocusItem; if (oldSubFocusItem) { - QQuickItemPrivate::get(oldSubFocusItem)->focus = false; + QQuickItemPrivate *priv = QQuickItemPrivate::get(oldSubFocusItem); + priv->focus = false; + priv->notifyChangeListeners(QQuickItemPrivate::Focus, &QQuickItemChangeListener::itemFocusChanged, oldSubFocusItem, reason); changed << oldSubFocusItem; } @@ -415,6 +417,7 @@ void QQuickDeliveryAgentPrivate::setFocusInScope(QQuickItem *scope, QQuickItem * #endif ) { itemPrivate->focus = true; + itemPrivate->notifyChangeListeners(QQuickItemPrivate::Focus, &QQuickItemChangeListener::itemFocusChanged, item, reason); changed << item; } } @@ -454,7 +457,7 @@ void QQuickDeliveryAgentPrivate::setFocusInScope(QQuickItem *scope, QQuickItem * emit rootItem->window()->focusObjectChanged(activeFocusItem); if (!changed.isEmpty()) - notifyFocusChangesRecur(changed.data(), changed.count() - 1); + notifyFocusChangesRecur(changed.data(), changed.count() - 1, reason); if (isSubsceneAgent) { auto da = QQuickWindowPrivate::get(rootItem->window())->deliveryAgent; qCDebug(lcFocus) << " delegating setFocusInScope to" << da; @@ -516,14 +519,18 @@ void QQuickDeliveryAgentPrivate::clearFocusInScope(QQuickItem *scope, QQuickItem if (item != rootItem && !(options & DontChangeSubFocusItem)) { QQuickItem *oldSubFocusItem = scopePrivate->subFocusItem; if (oldSubFocusItem && !(options & DontChangeFocusProperty)) { - QQuickItemPrivate::get(oldSubFocusItem)->focus = false; + QQuickItemPrivate *priv = QQuickItemPrivate::get(oldSubFocusItem); + priv->focus = false; + priv->notifyChangeListeners(QQuickItemPrivate::Focus, &QQuickItemChangeListener::itemFocusChanged, oldSubFocusItem, reason); changed << oldSubFocusItem; } QQuickItemPrivate::get(item)->updateSubFocusItem(scope, false); } else if (!(options & DontChangeFocusProperty)) { - QQuickItemPrivate::get(item)->focus = false; + QQuickItemPrivate *priv = QQuickItemPrivate::get(item); + priv->focus = false; + priv->notifyChangeListeners(QQuickItemPrivate::Focus, &QQuickItemChangeListener::itemFocusChanged, item, reason); changed << item; } @@ -550,7 +557,7 @@ void QQuickDeliveryAgentPrivate::clearFocusInScope(QQuickItem *scope, QQuickItem emit rootItem->window()->focusObjectChanged(activeFocusItem); if (!changed.isEmpty()) - notifyFocusChangesRecur(changed.data(), changed.count() - 1); + notifyFocusChangesRecur(changed.data(), changed.count() - 1, reason); if (oldActiveFocusItem == activeFocusItem) qCDebug(lcFocus) << "activeFocusItem remains" << activeFocusItem << "in" << q; @@ -566,24 +573,26 @@ void QQuickDeliveryAgentPrivate::clearFocusObject() clearFocusInScope(rootItem, QQuickItemPrivate::get(rootItem)->subFocusItem, Qt::OtherFocusReason); } -void QQuickDeliveryAgentPrivate::notifyFocusChangesRecur(QQuickItem **items, int remaining) +void QQuickDeliveryAgentPrivate::notifyFocusChangesRecur(QQuickItem **items, int remaining, Qt::FocusReason reason) { QPointer<QQuickItem> item(*items); if (remaining) - notifyFocusChangesRecur(items + 1, remaining - 1); + notifyFocusChangesRecur(items + 1, remaining - 1, reason); if (item) { QQuickItemPrivate *itemPrivate = QQuickItemPrivate::get(item); if (itemPrivate->notifiedFocus != itemPrivate->focus) { itemPrivate->notifiedFocus = itemPrivate->focus; + itemPrivate->notifyChangeListeners(QQuickItemPrivate::Focus, &QQuickItemChangeListener::itemFocusChanged, item, reason); emit item->focusChanged(itemPrivate->focus); } if (item && itemPrivate->notifiedActiveFocus != itemPrivate->activeFocus) { itemPrivate->notifiedActiveFocus = itemPrivate->activeFocus; itemPrivate->itemChange(QQuickItem::ItemActiveFocusHasChanged, itemPrivate->activeFocus); + itemPrivate->notifyChangeListeners(QQuickItemPrivate::Focus, &QQuickItemChangeListener::itemFocusChanged, item, reason); emit item->activeFocusChanged(itemPrivate->activeFocus); } } diff --git a/src/quick/util/qquickdeliveryagent_p_p.h b/src/quick/util/qquickdeliveryagent_p_p.h index 2a795f3e91..3ee295f133 100644 --- a/src/quick/util/qquickdeliveryagent_p_p.h +++ b/src/quick/util/qquickdeliveryagent_p_p.h @@ -95,7 +95,7 @@ public: void setFocusInScope(QQuickItem *scope, QQuickItem *item, Qt::FocusReason reason, FocusOptions = { }); void clearFocusInScope(QQuickItem *scope, QQuickItem *item, Qt::FocusReason reason, FocusOptions = { }); - static void notifyFocusChangesRecur(QQuickItem **item, int remaining); + static void notifyFocusChangesRecur(QQuickItem **item, int remaining, Qt::FocusReason reason); void clearFocusObject(); void updateFocusItemTransform(); diff --git a/src/quicktemplates2/qquickcontrol.cpp b/src/quicktemplates2/qquickcontrol.cpp index 71072063fa..cd5f0a539d 100644 --- a/src/quicktemplates2/qquickcontrol.cpp +++ b/src/quicktemplates2/qquickcontrol.cpp @@ -423,6 +423,8 @@ void QQuickControlPrivate::setContentItem_helper(QQuickItem *item, bool notify) QQuickItem *oldContentItem = contentItem; if (oldContentItem) { disconnect(oldContentItem, &QQuickItem::baselineOffsetChanged, this, &QQuickControlPrivate::updateBaselineOffset); + if (oldContentItem) + QQuickItemPrivate::get(oldContentItem)->removeItemChangeListener(this, QQuickControlPrivate::Focus); removeImplicitSizeListener(oldContentItem); } @@ -432,6 +434,10 @@ void QQuickControlPrivate::setContentItem_helper(QQuickItem *item, bool notify) if (item) { connect(contentItem.data(), &QQuickItem::baselineOffsetChanged, this, &QQuickControlPrivate::updateBaselineOffset); + // We need to update the control's focusReason when the contentItem receives or loses focus. Since focusPolicy + // (or other properties impacting focus handling, like QQuickItem::activeFocusOnTab) might change later, and + // since the content item might also change focus programmatically, we always have to listen for those events. + QQuickItemPrivate::get(item)->addItemChangeListener(this, QQuickControlPrivate::Focus); if (!item->parentItem()) item->setParentItem(q); if (componentComplete) @@ -852,6 +858,13 @@ void QQuickControlPrivate::itemDestroyed(QQuickItem *item) } } +void QQuickControlPrivate::itemFocusChanged(QQuickItem *item, Qt::FocusReason reason) +{ + Q_Q(QQuickControl); + if (item == contentItem || item == q) + q->setFocusReason(reason); +} + QQuickControl::QQuickControl(QQuickItem *parent) : QQuickItem(*(new QQuickControlPrivate), parent) { @@ -871,6 +884,8 @@ QQuickControl::~QQuickControl() Q_D(QQuickControl); d->removeImplicitSizeListener(d->background, QQuickControlPrivate::ImplicitSizeChanges | QQuickItemPrivate::Geometry); d->removeImplicitSizeListener(d->contentItem); + if (d->contentItem) + QQuickItemPrivate::get(d->contentItem)->removeItemChangeListener(d, QQuickItemPrivate::Focus); #if QT_CONFIG(accessibility) QAccessible::removeActivationObserver(d); #endif diff --git a/src/quicktemplates2/qquickcontrol_p_p.h b/src/quicktemplates2/qquickcontrol_p_p.h index cb2eb4190c..c488ab6398 100644 --- a/src/quicktemplates2/qquickcontrol_p_p.h +++ b/src/quicktemplates2/qquickcontrol_p_p.h @@ -177,6 +177,7 @@ public: void itemImplicitHeightChanged(QQuickItem *item) override; void itemGeometryChanged(QQuickItem *item, QQuickGeometryChange change, const QRectF &diff) override; void itemDestroyed(QQuickItem *item) override; + void itemFocusChanged(QQuickItem *item, Qt::FocusReason reason) override; virtual qreal getContentWidth() const; virtual qreal getContentHeight() const; diff --git a/tests/auto/quickcontrols2/focus/tst_focus.cpp b/tests/auto/quickcontrols2/focus/tst_focus.cpp index 2035215c93..e17031c396 100644 --- a/tests/auto/quickcontrols2/focus/tst_focus.cpp +++ b/tests/auto/quickcontrols2/focus/tst_focus.cpp @@ -329,7 +329,6 @@ void tst_focus::reason() QVERIFY(spinbox->hasFocus()); QVERIFY(spinbox->hasActiveFocus()); QCOMPARE(qApp->focusObject(), spinbox->contentItem()); - QEXPECT_FAIL("", "Events on content item not filtered - QTBUG-75862", Continue); QCOMPARE(editcombo->focusReason(), Qt::TabFocusReason); QCOMPARE(spinbox->focusReason(), Qt::TabFocusReason); spinbox->setFocusReason(Qt::NoFocusReason); @@ -338,7 +337,6 @@ void tst_focus::reason() QVERIFY(customText->hasFocus()); QVERIFY(customText->hasActiveFocus()); QCOMPARE(qApp->focusObject(), customText); - QEXPECT_FAIL("", "Events on content item not filtered - QTBUG-75862", Continue); QCOMPARE(spinbox->focusReason(), Qt::TabFocusReason); QCOMPARE(customText->focusReason(), Qt::TabFocusReason); customText->setFocusReason(Qt::NoFocusReason); @@ -377,12 +375,10 @@ void tst_focus::reason() QTest::keyClick(&view, Qt::Key_Tab, Qt::ShiftModifier); QVERIFY(spinbox->hasFocus()); - QEXPECT_FAIL("", "Events on content item not filtered - QTBUG-75862", Continue); QCOMPARE(spinbox->focusReason(), Qt::BacktabFocusReason); QTest::keyClick(&view, Qt::Key_Tab, Qt::ShiftModifier); QVERIFY(editcombo->hasFocus()); - QEXPECT_FAIL("", "Events on content item not filtered - QTBUG-75862", Continue); QCOMPARE(editcombo->focusReason(), Qt::BacktabFocusReason); QTest::keyClick(&view, Qt::Key_Tab, Qt::ShiftModifier); @@ -395,7 +391,6 @@ void tst_focus::reason() QVERIFY(editcombo->contentItem()->hasFocus()); QVERIFY(editcombo->hasActiveFocus()); QVERIFY(editcombo->contentItem()->hasActiveFocus()); - QEXPECT_FAIL("", "Events on content item not filtered - QTBUG-75862", Continue); QCOMPARE(editcombo->focusReason(), Qt::MouseFocusReason); editcombo->setFocusReason(Qt::NoFocusReason); @@ -404,7 +399,6 @@ void tst_focus::reason() QVERIFY(combobox->hasFocus()); QVERIFY(combobox->hasActiveFocus()); - QEXPECT_FAIL("", "Events on content item not filtered - QTBUG-75862", Continue); QCOMPARE(editcombo->focusReason(), Qt::MouseFocusReason); QCOMPARE(combobox->focusReason(), Qt::MouseFocusReason); combobox->setFocusReason(Qt::NoFocusReason); @@ -413,23 +407,19 @@ void tst_focus::reason() QVERIFY(spinbox->hasFocus()); QVERIFY(spinbox->hasActiveFocus()); QCOMPARE(combobox->focusReason(), Qt::MouseFocusReason); - QEXPECT_FAIL("", "Events on content item not filtered - QTBUG-75862", Continue); QCOMPARE(spinbox->focusReason(), Qt::MouseFocusReason); spinbox->setFocusReason(Qt::NoFocusReason); QTest::mouseClick(&view, Qt::LeftButton, {}, itemCenter(customText)); QTRY_VERIFY2(customText->contentItem()->hasFocus(), qPrintable(qApp->focusObject()->objectName())); QVERIFY(customText->contentItem()->hasActiveFocus()); - QEXPECT_FAIL("", "Events on content item not filtered - QTBUG-75862", Continue); QCOMPARE(spinbox->focusReason(), Qt::MouseFocusReason); - QEXPECT_FAIL("", "Events on content item not filtered - QTBUG-75862", Continue); QCOMPARE(customText->focusReason(), Qt::MouseFocusReason); customText->setFocusReason(Qt::NoFocusReason); QTest::mouseClick(&view, Qt::LeftButton, {}, itemCenter(customItem)); QVERIFY(customItem->hasFocus()); QVERIFY(customItem->hasActiveFocus()); - QEXPECT_FAIL("", "Events on content item not filtered - QTBUG-75862", Continue); QCOMPARE(customText->focusReason(), Qt::MouseFocusReason); QCOMPARE(customItem->focusReason(), Qt::MouseFocusReason); customItem->setFocusReason(Qt::NoFocusReason); |