Protect tab focus chain from infinite loops when item is invisible

Make sure to start the search for a potential focus item on an item that
will be visited again later, otherwise we'll loop for ever.

Fixes: QTBUG-68271
Change-Id: Icb330e4e726132511810027a33b9fb346c7fa131
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>
Reviewed-by: Gabriel de Dietrich <gabriel.dedietrich@qt.io>

3 files changed, 43 insertions, 2 deletions

diff --git a/src/quick/items/qquickitem.cpp b/src/quick/items/qquickitem.cpp
index 6b5d595c26..8e2d32a818 100644
--- a/src/quick/items/qquickitem.cpp
+++ b/src/quick/items/qquickitem.cpp
@@ -2566,8 +2566,19 @@ QQuickItem* QQuickItemPrivate::nextPrevItemInTabFocusChain(QQuickItem *item, boo
             from = item->parentItem();
     }
     bool skip = false;
-    QQuickItem * startItem = item;
-    QQuickItem * firstFromItem = from;
+
+    QQuickItem *startItem = item;
+    // 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,
+    // since the protection only works if we encounter the first item again.
+    while (startItem && !startItem->isVisible()) {
+        startItem = startItem->parentItem();
+    }
+    if (!startItem)
+        return item;
+
+    QQuickItem *firstFromItem = from;
     QQuickItem *current = item;
     qCDebug(DBG_FOCUS) << "QQuickItemPrivate::nextPrevItemInTabFocusChain: startItem:" << startItem;
     qCDebug(DBG_FOCUS) << "QQuickItemPrivate::nextPrevItemInTabFocusChain: firstFromItem:" << firstFromItem;
diff --git a/tests/auto/quick/qquickitem2/data/activeFocusOnTab_infiniteLoop.qml b/tests/auto/quick/qquickitem2/data/activeFocusOnTab_infiniteLoop.qml
new file mode 100644
index 0000000000..889e480f3b
--- /dev/null
+++ b/tests/auto/quick/qquickitem2/data/activeFocusOnTab_infiniteLoop.qml
@@ -0,0 +1,13 @@
+import QtQuick 2.6
+
+Item {
+    visible: true
+    Item {
+        visible: false
+        Item {
+            objectName: "hiddenChild"
+            activeFocusOnTab: true
+            focus: true
+        }
+    }
+}
diff --git a/tests/auto/quick/qquickitem2/tst_qquickitem.cpp b/tests/auto/quick/qquickitem2/tst_qquickitem.cpp
index f0f5873ace..7107f4d995 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();
 
     void nextItemInFocusChain();
     void nextItemInFocusChain2();
@@ -1023,6 +1024,22 @@ void tst_QQuickItem::activeFocusOnTab10()
     delete window;
 }
 
+void tst_QQuickItem::activeFocusOnTab_infiniteLoop()
+{
+    // see QTBUG-68271
+    // create a window where the currently focused item is not visible
+    QScopedPointer<QQuickView>window(new QQuickView());
+    window->setSource(testFileUrl("activeFocusOnTab_infiniteLoop.qml"));
+    window->show();
+    auto *hiddenChild = findItem<QQuickItem>(window->rootObject(), "hiddenChild");
+    QVERIFY(hiddenChild);
+
+    // move the focus - this used to result in an infinite loop
+    auto *item = hiddenChild->nextItemInFocusChain();
+    // focus is moved to the root object since there is no other candidate
+    QCOMPARE(item, window->rootObject());
+}
+
 void tst_QQuickItem::nextItemInFocusChain()
 {
     if (!qt_tab_all_widgets())