From 63f83fbd095415145ad124c7215f07a4c3c6038f Mon Sep 17 00:00:00 2001
From: Liang Qi <liang.qi@digia.com>
Date: Mon, 27 May 2013 10:19:35 +0200
Subject: [PATCH] Prevent tab focus from wrapping endlessly

If there was no item that accepted focus, it would go into an
endless loop.

This also changes the default behavior of QQuickWindow. When there is not
any activeFocusItem in the whole window, it means the contentItem got
focused. The Tab/BackTab key will now focus the next item in the tab focus
chain.

Autotest is included.

Done-with: Frederik Gladhorn <frederik.gladhorn@digia.com>
Task-number: QTBUG-31344
Change-Id: I854292f89a327c493eec21969907c94aa9cfddcb
Reviewed-by: Jens Bache-Wiig <jens.bache-wiig@digia.com>
Reviewed-by: Gabriel de Dietrich <gabriel.dedietrich@digia.com>
---
 src/quick/items/qquickitem.cpp                | 32 ++++++-
 .../qquickitem2/data/activeFocusOnTab7.qml    | 36 ++++++++
 .../qquickitem2/data/activeFocusOnTab8.qml    | 36 ++++++++
 .../auto/quick/qquickitem2/tst_qquickitem.cpp | 87 +++++++++++++++++++
 4 files changed, 187 insertions(+), 4 deletions(-)
 create mode 100644 tests/auto/quick/qquickitem2/data/activeFocusOnTab7.qml
 create mode 100644 tests/auto/quick/qquickitem2/data/activeFocusOnTab8.qml

diff --git a/src/quick/items/qquickitem.cpp b/src/quick/items/qquickitem.cpp
index d6a663ab77..58e1612e19 100644
--- a/src/quick/items/qquickitem.cpp
+++ b/src/quick/items/qquickitem.cpp
@@ -2050,7 +2050,10 @@ bool QQuickItemPrivate::canAcceptTabFocus(QQuickItem *item)
 {
     bool result = true;
 
-    if (item->window() && item == item->window()->contentItem())
+    if (!item->window())
+        return false;
+
+    if (item == item->window()->contentItem())
         return true;
 
 #ifndef QT_NO_ACCESSIBILITY
@@ -2093,7 +2096,6 @@ bool QQuickItemPrivate::focusNextPrev(QQuickItem *item, bool forward)
 QQuickItem* QQuickItemPrivate::nextPrevItemInTabFocusChain(QQuickItem *item, bool forward)
 {
     Q_ASSERT(item);
-    Q_ASSERT(item->activeFocusOnTab());
 
     bool all = QQuickItemPrivate::qt_tab_all_widgets();
 
@@ -2107,6 +2109,10 @@ QQuickItem* QQuickItemPrivate::nextPrevItemInTabFocusChain(QQuickItem *item, boo
             from = item->parentItem();
     }
     bool skip = false;
+    const QQuickItem * const contentItem = item->window()->contentItem();
+    const QQuickItem * const originalItem = item;
+    QQuickItem * startItem = item;
+    QQuickItem * firstFromItem = from;
     QQuickItem *current = item;
     do {
         skip = false;
@@ -2157,8 +2163,25 @@ QQuickItem* QQuickItemPrivate::nextPrevItemInTabFocusChain(QQuickItem *item, boo
                     skip = true;
             }
         }
-
         from = last;
+        if (current == startItem && from == firstFromItem) {
+            // wrapped around, avoid endless loops
+            if (originalItem == contentItem) {
+#ifdef FOCUS_DEBUG
+                qDebug() << "QQuickItemPrivate::nextPrevItemInTabFocusChain: looped, return contentItem";
+#endif
+                return item->window()->contentItem();
+            } else {
+#ifdef FOCUS_DEBUG
+                qDebug() << "QQuickItemPrivate::nextPrevItemInTabFocusChain: looped, return " << startItem;
+#endif
+                return startItem;
+            }
+        }
+        if (!firstFromItem) { //start from root
+            startItem = current;
+            firstFromItem = from;
+        }
     } while (skip || !current->activeFocusOnTab() || !current->isEnabled() || !current->isVisible()
                   || !(all || QQuickItemPrivate::canAcceptTabFocus(current)));
 
@@ -4375,7 +4398,8 @@ void QQuickItemPrivate::deliverKeyEvent(QKeyEvent *e)
         return;
 
     //only care about KeyPress now
-    if (q->activeFocusOnTab() && e->type() == QEvent::KeyPress) {
+    if ((q == q->window()->contentItem() || q->activeFocusOnTab())
+            && e->type() == QEvent::KeyPress) {
         bool res = false;
         if (!(e->modifiers() & (Qt::ControlModifier | Qt::AltModifier))) {  //### Add MetaModifier?
             if (e->key() == Qt::Key_Backtab
diff --git a/tests/auto/quick/qquickitem2/data/activeFocusOnTab7.qml b/tests/auto/quick/qquickitem2/data/activeFocusOnTab7.qml
new file mode 100644
index 0000000000..e81d9be950
--- /dev/null
+++ b/tests/auto/quick/qquickitem2/data/activeFocusOnTab7.qml
@@ -0,0 +1,36 @@
+import QtQuick 2.1
+
+Item {
+    id: main
+    objectName: "main"
+    width: 300
+    height: 300
+    Item {
+        id: button1
+        objectName: "button1"
+        width: 300
+        height: 150
+        activeFocusOnTab: true
+        Accessible.role: Accessible.Button
+        Rectangle {
+            anchors.fill: parent
+            color: parent.activeFocus ? "red" : "black"
+        }
+        anchors.top: parent.top
+        anchors.left: parent.left
+    }
+    Item {
+        id: button2
+        objectName: "button2"
+        width: 300
+        height: 150
+        activeFocusOnTab: true
+        Accessible.role: Accessible.Button
+        Rectangle {
+            anchors.fill: parent
+            color: parent.activeFocus ? "red" : "black"
+        }
+        anchors.bottom: parent.bottom
+        anchors.left: parent.left
+    }
+}
diff --git a/tests/auto/quick/qquickitem2/data/activeFocusOnTab8.qml b/tests/auto/quick/qquickitem2/data/activeFocusOnTab8.qml
new file mode 100644
index 0000000000..641a39c1fa
--- /dev/null
+++ b/tests/auto/quick/qquickitem2/data/activeFocusOnTab8.qml
@@ -0,0 +1,36 @@
+import QtQuick 2.1
+
+Item {
+    id: main
+    objectName: "main"
+    width: 300
+    height: 300
+    Item {
+        id: button1
+        objectName: "button1"
+        width: 300
+        height: 150
+        activeFocusOnTab: true
+        Accessible.role: Accessible.Table
+        Rectangle {
+            anchors.fill: parent
+            color: parent.activeFocus ? "red" : "black"
+        }
+        anchors.top: parent.top
+        anchors.left: parent.left
+    }
+    Item {
+        id: button2
+        objectName: "button2"
+        width: 300
+        height: 150
+        activeFocusOnTab: true
+        Accessible.role: Accessible.Table
+        Rectangle {
+            anchors.fill: parent
+            color: parent.activeFocus ? "red" : "black"
+        }
+        anchors.bottom: parent.bottom
+        anchors.left: parent.left
+    }
+}
diff --git a/tests/auto/quick/qquickitem2/tst_qquickitem.cpp b/tests/auto/quick/qquickitem2/tst_qquickitem.cpp
index 9a6bed6dbe..992e81aa64 100644
--- a/tests/auto/quick/qquickitem2/tst_qquickitem.cpp
+++ b/tests/auto/quick/qquickitem2/tst_qquickitem.cpp
@@ -71,6 +71,8 @@ private slots:
     void activeFocusOnTab4();
     void activeFocusOnTab5();
     void activeFocusOnTab6();
+    void activeFocusOnTab7();
+    void activeFocusOnTab8();
 
     void nextItemInFocusChain();
     void nextItemInFocusChain2();
@@ -747,6 +749,91 @@ void tst_QQuickItem::activeFocusOnTab6()
     delete window;
 }
 
+void tst_QQuickItem::activeFocusOnTab7()
+{
+    if (qt_tab_all_widgets())
+        QSKIP("This function doesn't support iterating all.");
+
+    QQuickView *window = new QQuickView(0);
+    window->setBaseSize(QSize(300,300));
+
+    window->setSource(testFileUrl("activeFocusOnTab7.qml"));
+    window->show();
+    window->requestActivate();
+    QVERIFY(QTest::qWaitForWindowActive(window));
+    QVERIFY(QGuiApplication::focusWindow() == window);
+
+    QQuickItem *item = findItem<QQuickItem>(window->rootObject(), "button1");
+    QVERIFY(item);
+    item->forceActiveFocus();
+    QVERIFY(item->hasActiveFocus());
+
+    // Tab: button1->button1
+    QKeyEvent key(QEvent::KeyPress, Qt::Key_Tab, Qt::NoModifier, "", false, 1);
+    QGuiApplication::sendEvent(window, &key);
+    QVERIFY(!key.isAccepted());
+
+    QVERIFY(item->hasActiveFocus());
+
+    // BackTab: button1->button1
+    key = QKeyEvent(QEvent::KeyPress, Qt::Key_Tab, Qt::ShiftModifier, "", false, 1);
+    QGuiApplication::sendEvent(window, &key);
+    QVERIFY(!key.isAccepted());
+
+    QVERIFY(item->hasActiveFocus());
+
+    delete window;
+}
+
+void tst_QQuickItem::activeFocusOnTab8()
+{
+    QQuickView *window = new QQuickView(0);
+    window->setBaseSize(QSize(300,300));
+
+    window->setSource(testFileUrl("activeFocusOnTab8.qml"));
+    window->show();
+    window->requestActivate();
+    QVERIFY(QTest::qWaitForWindowActive(window));
+    QVERIFY(QGuiApplication::focusWindow() == window);
+
+    QQuickItem *content = window->contentItem();
+    QVERIFY(content);
+    QVERIFY(content->hasActiveFocus());
+
+    QQuickItem *button1 = findItem<QQuickItem>(window->rootObject(), "button1");
+    QVERIFY(button1);
+    QVERIFY(!button1->hasActiveFocus());
+
+    QQuickItem *button2 = findItem<QQuickItem>(window->rootObject(), "button2");
+    QVERIFY(button2);
+    QVERIFY(!button2->hasActiveFocus());
+
+    // Tab: contentItem->button1
+    QKeyEvent key(QEvent::KeyPress, Qt::Key_Tab, Qt::NoModifier, "", false, 1);
+    QGuiApplication::sendEvent(window, &key);
+    QVERIFY(key.isAccepted());
+
+    QVERIFY(button1->hasActiveFocus());
+
+    // Tab: button1->button2
+    key = QKeyEvent(QEvent::KeyPress, Qt::Key_Tab, Qt::NoModifier, "", false, 1);
+    QGuiApplication::sendEvent(window, &key);
+    QVERIFY(key.isAccepted());
+
+    QVERIFY(button2->hasActiveFocus());
+    QVERIFY(!button1->hasActiveFocus());
+
+    // BackTab: button2->button1
+    key = QKeyEvent(QEvent::KeyPress, Qt::Key_Tab, Qt::ShiftModifier, "", false, 1);
+    QGuiApplication::sendEvent(window, &key);
+    QVERIFY(key.isAccepted());
+
+    QVERIFY(button1->hasActiveFocus());
+    QVERIFY(!button2->hasActiveFocus());
+
+    delete window;
+}
+
 void tst_QQuickItem::nextItemInFocusChain()
 {
     if (!qt_tab_all_widgets())
-- 
GitLab