QQuickItem: prevent endless loop in focus tab chain

Since the commit a18ab2a3822e0d, we promote the [startItem] in focus
tab chain when it is invisible to prevent endless loop. However the
problem still happen if the [startItem] is equal to [firstFromItem]

Fixes it by compare the [current] item with the original start item

Fixes: QTBUG-81510
Change-Id: Iae0207f39e2b8c4fc6ed0cf36f0a855668accfba
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>
Reviewed-by: Liang Qi <liang.qi@qt.io>
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>

3 files changed, 32 insertions, 3 deletions

diff --git a/src/quick/items/qquickitem.cpp b/src/quick/items/qquickitem.cpp
index 3785abc450..11c1f12e75 100644
--- a/src/quick/items/qquickitem.cpp
+++ b/src/quick/items/qquickitem.cpp
@@ -2568,6 +2568,7 @@ QQuickItem* QQuickItemPrivate::nextPrevItemInTabFocusChain(QQuickItem *item, boo
     bool skip = false;
 
     QQuickItem *startItem = item;
+    QQuickItem *originalStartItem = startItem;
     // Protect from endless loop:
     // If we start on an invisible item we will not find it again.
     // If there is no other item which can become the focus item, we have a forever loop,
@@ -2643,7 +2644,12 @@ QQuickItem* QQuickItemPrivate::nextPrevItemInTabFocusChain(QQuickItem *item, boo
             }
         }
         from = last;
-        if (current == startItem && from == firstFromItem) {
+        // if [from] item is equal to [firstFromItem], means we have traversed one path and
+        // jump back to parent of the chain, and then we have to check whether we have
+        // traversed all of the chain (by compare the [current] item with [startItem])
+        // Since the [startItem] might be promoted to its parent if it is invisible,
+        // we still have to check [current] item with original start item
+        if ((current == startItem || current == originalStartItem) && from == firstFromItem) {
             // wrapped around, avoid endless loops
             if (item == contentItem) {
                 qCDebug(DBG_FOCUS) << "QQuickItemPrivate::nextPrevItemInTabFocusChain: looped, return contentItem";
diff --git a/tests/auto/quick/qquickitem2/data/activeFocusOnTab_infiniteLoop2.qml b/tests/auto/quick/qquickitem2/data/activeFocusOnTab_infiniteLoop2.qml
new file mode 100644
index 0000000000..042f408753
--- /dev/null
+++ b/tests/auto/quick/qquickitem2/data/activeFocusOnTab_infiniteLoop2.qml
@@ -0,0 +1,12 @@
+import QtQuick 2.14
+
+Item {
+    width: 400
+    height: 200
+    Item {
+        objectName: "hiddenChild"
+        focus: true
+        activeFocusOnTab: true
+        visible: false
+    }
+}
diff --git a/tests/auto/quick/qquickitem2/tst_qquickitem.cpp b/tests/auto/quick/qquickitem2/tst_qquickitem.cpp
index 399535cfa6..767994ec7d 100644
--- a/tests/auto/quick/qquickitem2/tst_qquickitem.cpp
+++ b/tests/auto/quick/qquickitem2/tst_qquickitem.cpp
@@ -64,6 +64,7 @@ private slots:
     void activeFocusOnTab8();
     void activeFocusOnTab9();
     void activeFocusOnTab10();
+    void activeFocusOnTab_infiniteLoop_data();
     void activeFocusOnTab_infiniteLoop();
 
     void nextItemInFocusChain();
@@ -1025,12 +1026,20 @@ void tst_QQuickItem::activeFocusOnTab10()
     delete window;
 }
 
+void tst_QQuickItem::activeFocusOnTab_infiniteLoop_data()
+{
+    QTest::addColumn<QUrl>("source");
+    QTest::newRow("infiniteLoop") << testFileUrl("activeFocusOnTab_infiniteLoop.qml");  // QTBUG-68271
+    QTest::newRow("infiniteLoop2") << testFileUrl("activeFocusOnTab_infiniteLoop2.qml");// QTBUG-81510
+}
+
 void tst_QQuickItem::activeFocusOnTab_infiniteLoop()
 {
-    // see QTBUG-68271
+    QFETCH(QUrl, source);
+
     // create a window where the currently focused item is not visible
     QScopedPointer<QQuickView>window(new QQuickView());
-    window->setSource(testFileUrl("activeFocusOnTab_infiniteLoop.qml"));
+    window->setSource(source);
     window->show();
     auto *hiddenChild = findItem<QQuickItem>(window->rootObject(), "hiddenChild");
     QVERIFY(hiddenChild);
@@ -1039,6 +1048,8 @@ void tst_QQuickItem::activeFocusOnTab_infiniteLoop()
     auto *item = hiddenChild->nextItemInFocusChain();
     // focus is moved to the root object since there is no other candidate
     QCOMPARE(item, window->rootObject());
+    item = hiddenChild->nextItemInFocusChain(false);
+    QCOMPARE(item, window->rootObject());
 }
 
 void tst_QQuickItem::nextItemInFocusChain()