From ab253c6bc7905c1d734e58d4d673ca45c5384967 Mon Sep 17 00:00:00 2001 From: Shawn Rutledge Date: Fri, 16 Apr 2021 14:16:37 +0200 Subject: QQuickItem::forceActiveFocus(): actually force active focus It was trying to get by with setFocus() but that doesn't always work, in cases when the item's d->focus flag is true (leftover state) but it doesn't actually have focus anymore after a reparenting scenario. Item.focus represents the intention to be focused when possible, and does not necessarily change due to environmental circumstances, such as having its parent reparented. QQuickItem::setFocus(true) returns early if the new requested focus state is the same as the stored d->focus; so it was not enough for foceActiveFocus() to call only setFocus(). In the bug, TextInput and Loader both get stuck in the state d->focus == true, so forceActiveFocus() did not do anything before. Fixes: QTBUG-89736 Change-Id: Ib7f4caccf81b60a02e2655332b64efba4d1fd7cf Reviewed-by: Richard Moe Gustavsen (cherry picked from commit a8be17a1472c6f504c0c40f68fdc13df035ac4b4) Reviewed-by: Qt Cherry-pick Bot --- src/quick/items/qquickitem.cpp | 12 +++++ .../focusableItemReparentedToLoadedComponent.qml | 51 ++++++++++++++++++++++ tests/auto/quick/qquickitem2/tst_qquickitem.cpp | 31 +++++++++++++ 3 files changed, 94 insertions(+) create mode 100644 tests/auto/quick/qquickitem2/data/focusableItemReparentedToLoadedComponent.qml diff --git a/src/quick/items/qquickitem.cpp b/src/quick/items/qquickitem.cpp index 82f4fac2bf..464076cbf8 100644 --- a/src/quick/items/qquickitem.cpp +++ b/src/quick/items/qquickitem.cpp @@ -4798,14 +4798,26 @@ void QQuickItem::forceActiveFocus() void QQuickItem::forceActiveFocus(Qt::FocusReason reason) { + Q_D(QQuickItem); setFocus(true, reason); QQuickItem *parent = parentItem(); + QQuickItem *scope = nullptr; while (parent) { if (parent->flags() & QQuickItem::ItemIsFocusScope) { parent->setFocus(true, reason); + if (!scope) + scope = parent; } parent = parent->parentItem(); } + // In certain reparenting scenarios, d->focus might be true and the scope + // might also have focus, so that setFocus() returns early without actually + // acquiring active focus, because it thinks it already has it. In that + // case, try to set the DeliveryAgent's active focus. (QTBUG-89736). + if (scope && !d->activeFocus) { + if (auto da = d->deliveryAgentPrivate()) + da->setFocusInScope(scope, this, Qt::OtherFocusReason); + } } /*! diff --git a/tests/auto/quick/qquickitem2/data/focusableItemReparentedToLoadedComponent.qml b/tests/auto/quick/qquickitem2/data/focusableItemReparentedToLoadedComponent.qml new file mode 100644 index 0000000000..a690c4243b --- /dev/null +++ b/tests/auto/quick/qquickitem2/data/focusableItemReparentedToLoadedComponent.qml @@ -0,0 +1,51 @@ +import QtQuick 2.12 + +Item { + width: 240; height: 240 + Loader { + id: loader + sourceComponent: surfaceParent + anchors.fill: parent + + onStatusChanged: { + if (status === Loader.Ready) { + holder.create() + holder.item.parent = item + } else if (status === Loader.Null){ + holder.item.parent = null + } + } + } + + property var holder: QtObject { + property bool created: false + function create() + { + if (!created) + surfaceComponent.createObject(item) + created = true + } + + property Item item: Item { + anchors.fill: parent + Component { + id: surfaceComponent + Item { + anchors.fill: parent + TextInput { + width: parent.width + font.pixelSize: 40 + text: "focus me" + } + } + } + } + } + + Component { + id: surfaceParent + Rectangle { + anchors.fill: parent + } + } +} diff --git a/tests/auto/quick/qquickitem2/tst_qquickitem.cpp b/tests/auto/quick/qquickitem2/tst_qquickitem.cpp index ec3ce69433..a33290153a 100644 --- a/tests/auto/quick/qquickitem2/tst_qquickitem.cpp +++ b/tests/auto/quick/qquickitem2/tst_qquickitem.cpp @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -75,6 +76,7 @@ private slots: void qtbug_50516(); void qtbug_50516_2_data(); void qtbug_50516_2(); + void focusableItemReparentedToLoadedComponent(); void keys(); #if QT_CONFIG(shortcut) @@ -1400,6 +1402,35 @@ void tst_QQuickItem::qtbug_50516_2() delete window; } +void tst_QQuickItem::focusableItemReparentedToLoadedComponent() // QTBUG-89736 +{ + QQuickView window; + window.setSource(testFileUrl("focusableItemReparentedToLoadedComponent.qml")); + window.show(); + QVERIFY(QTest::qWaitForWindowActive(&window)); + QCOMPARE(QGuiApplication::focusWindow(), &window); + QQuickLoader *loader = window.rootObject()->findChild(); + QVERIFY(loader); + QTRY_VERIFY(loader->status() == QQuickLoader::Ready); + QQuickTextInput *textInput = window.rootObject()->findChild(); + QVERIFY(textInput); + + // click to focus + QTest::mouseClick(&window, Qt::LeftButton, Qt::NoModifier, {10, 10}); + QTRY_VERIFY(textInput->hasActiveFocus()); + + // unload and reload + auto component = loader->sourceComponent(); + loader->resetSourceComponent(); + QTRY_VERIFY(loader->status() == QQuickLoader::Null); + loader->setSourceComponent(component); + QTRY_VERIFY(loader->status() == QQuickLoader::Ready); + + // click to focus again + QTest::mouseClick(&window, Qt::LeftButton, Qt::NoModifier, {10, 10}); + QTRY_VERIFY(textInput->hasActiveFocus()); +} + void tst_QQuickItem::keys() { QQuickView *window = new QQuickView(nullptr); -- cgit v1.2.3