QQuickWindow: Only send focus events to items that actually gain focus

Sending focus events may result in further changes to which item actually has
focus, so we cannot blindly send the focus events without first checking that it
hasn't subsequently changed.

To accomplish this, we delay sending events as long as possible, ensuring that
all necessary bookkeeping is done first to ensure internal consistency.

Task-number: QTBUG-40145
Change-Id: I7d93b3f8e3fea2ecce2151c88c29601deda12453
Reviewed-by: Shawn Rutledge <shawn.rutledge@theqtcompany.com>

3 files changed, 98 insertions, 6 deletions

diff --git a/src/quick/items/qquickwindow.cpp b/src/quick/items/qquickwindow.cpp
index ef534fe35d..14e7915dba 100644
--- a/src/quick/items/qquickwindow.cpp
+++ b/src/quick/items/qquickwindow.cpp
@@ -785,8 +785,10 @@ void QQuickWindowPrivate::setFocusInScope(QQuickItem *scope, QQuickItem *item, Q
     QQuickItemPrivate *scopePrivate = scope ? QQuickItemPrivate::get(scope) : 0;
     QQuickItemPrivate *itemPrivate = QQuickItemPrivate::get(item);
 
+    QQuickItem *oldActiveFocusItem = 0;
     QQuickItem *currentActiveFocusItem = activeFocusItem;
     QQuickItem *newActiveFocusItem = 0;
+    bool sendFocusIn = false;
 
     lastFocusReason = reason;
 
@@ -794,7 +796,6 @@ void QQuickWindowPrivate::setFocusInScope(QQuickItem *scope, QQuickItem *item, Q
 
     // Does this change the active focus?
     if (item == contentItem || scopePrivate->activeFocus) {
-        QQuickItem *oldActiveFocusItem = 0;
         oldActiveFocusItem = activeFocusItem;
         if (item->isEnabled()) {
             newActiveFocusItem = item;
@@ -813,8 +814,6 @@ void QQuickWindowPrivate::setFocusInScope(QQuickItem *scope, QQuickItem *item, Q
 #endif
 
             activeFocusItem = 0;
-            QFocusEvent event(QEvent::FocusOut, reason);
-            q->sendEvent(oldActiveFocusItem, &event);
 
             QQuickItem *afi = oldActiveFocusItem;
             while (afi && afi != scope) {
@@ -859,7 +858,19 @@ void QQuickWindowPrivate::setFocusInScope(QQuickItem *scope, QQuickItem *item, Q
             afi = afi->parentItem();
         }
         updateFocusItemTransform();
+        sendFocusIn = true;
+    }
+
+    // Now that all the state is changed, emit signals & events
+    // We must do this last, as this process may result in further changes to
+    // focus.
+    if (oldActiveFocusItem) {
+        QFocusEvent event(QEvent::FocusOut, reason);
+        q->sendEvent(oldActiveFocusItem, &event);
+    }
 
+    // Make sure that the FocusOut didn't result in another focus change.
+    if (sendFocusIn && activeFocusItem == newActiveFocusItem) {
         QFocusEvent event(QEvent::FocusIn, reason);
         q->sendEvent(newActiveFocusItem, &event);
     }
@@ -912,9 +923,6 @@ void QQuickWindowPrivate::clearFocusInScope(QQuickItem *scope, QQuickItem *item,
         activeFocusItem = 0;
 
         if (oldActiveFocusItem) {
-            QFocusEvent event(QEvent::FocusOut, reason);
-            q->sendEvent(oldActiveFocusItem, &event);
-
             QQuickItem *afi = oldActiveFocusItem;
             while (afi && afi != scope) {
                 if (QQuickItemPrivate::get(afi)->activeFocus) {
@@ -944,7 +952,18 @@ void QQuickWindowPrivate::clearFocusInScope(QQuickItem *scope, QQuickItem *item,
         Q_ASSERT(newActiveFocusItem == scope);
         activeFocusItem = scope;
         updateFocusItemTransform();
+    }
+
+    // Now that all the state is changed, emit signals & events
+    // We must do this last, as this process may result in further changes to
+    // focus.
+    if (oldActiveFocusItem) {
+        QFocusEvent event(QEvent::FocusOut, reason);
+        q->sendEvent(oldActiveFocusItem, &event);
+    }
 
+    // Make sure that the FocusOut didn't result in another focus change.
+    if (newActiveFocusItem && activeFocusItem == newActiveFocusItem) {
         QFocusEvent event(QEvent::FocusIn, reason);
         q->sendEvent(newActiveFocusItem, &event);
     }
diff --git a/tests/auto/quick/qquicktextinput/data/focusOnlyOneOnPress.qml b/tests/auto/quick/qquicktextinput/data/focusOnlyOneOnPress.qml
new file mode 100644
index 0000000000..037b36c8ff
--- /dev/null
+++ b/tests/auto/quick/qquicktextinput/data/focusOnlyOneOnPress.qml
@@ -0,0 +1,32 @@
+import QtQuick 2.2
+
+Rectangle {
+    width: 400
+    height: 400
+
+    Column {
+        spacing: 5
+        TextInput {
+            objectName: "first"
+            onEditingFinished: second.focus = true
+            width: 100
+            Rectangle { anchors.fill: parent; color: parent.activeFocus ? "red" : "blue"; opacity: 0.3 }
+        }
+        TextInput {
+            id: second
+            objectName: "second"
+            onEditingFinished: third.focus = true
+            width: 100
+            Rectangle { anchors.fill: parent; color: parent.activeFocus ? "red" : "blue"; opacity: 0.3 }
+        }
+        TextInput {
+            objectName: "third"
+            id: third
+            width: 100
+            Rectangle { anchors.fill: parent; color: parent.activeFocus ? "red" : "blue"; opacity: 0.3 }
+        }
+        Component.onCompleted: {
+            second.focus = true
+        }
+    }
+}
diff --git a/tests/auto/quick/qquicktextinput/tst_qquicktextinput.cpp b/tests/auto/quick/qquicktextinput/tst_qquicktextinput.cpp
index 70d3906ff3..e57a95184c 100644
--- a/tests/auto/quick/qquicktextinput/tst_qquicktextinput.cpp
+++ b/tests/auto/quick/qquicktextinput/tst_qquicktextinput.cpp
@@ -158,6 +158,7 @@ private slots:
 #endif
     void readOnly();
     void focusOnPress();
+    void focusOnPressOnlyOneItem();
 
     void openInputPanel();
     void setHAlignClearCache();
@@ -3457,6 +3458,46 @@ void tst_qquicktextinput::focusOnPress()
     QTest::mouseRelease(&window, Qt::LeftButton, noModifiers);
 }
 
+void tst_qquicktextinput::focusOnPressOnlyOneItem()
+{
+    QQuickView window(testFileUrl("focusOnlyOneOnPress.qml"));
+    window.show();
+    window.requestActivate();
+    QTest::qWaitForWindowActive(&window);
+
+    QQuickTextInput *first = window.rootObject()->findChild<QQuickTextInput*>("first");
+    QQuickTextInput *second = window.rootObject()->findChild<QQuickTextInput*>("second");
+    QQuickTextInput *third = window.rootObject()->findChild<QQuickTextInput*>("third");
+
+    // second is focused onComplete
+    QVERIFY(second->hasActiveFocus());
+
+    // and first will try focus when we press it
+    QVERIFY(first->focusOnPress());
+
+    // write some text to start editing
+    QTest::keyClick(&window, Qt::Key_A);
+
+    // click the first input. naturally, we are giving focus on press, but
+    // second's editingFinished also attempts to assign focus. lastly, focus
+    // should bounce back to second from first's editingFinished signal.
+    //
+    // this is a contrived example to be sure, but at the end of this, the
+    // important thing is that only one thing should have activeFocus.
+    Qt::KeyboardModifiers noModifiers = 0;
+    QTest::mousePress(&window, Qt::LeftButton, noModifiers, QPoint(10, 10));
+
+    // make sure the press is processed.
+    QGuiApplication::processEvents();
+
+    QVERIFY(second->hasActiveFocus()); // make sure it's still there
+    QVERIFY(!third->hasActiveFocus()); // make sure it didn't end up anywhere else
+    QVERIFY(!first->hasActiveFocus()); // make sure it didn't end up anywhere else
+
+    // reset state
+    QTest::mouseRelease(&window, Qt::LeftButton, noModifiers, QPoint(10, 10));
+}
+
 void tst_qquicktextinput::openInputPanel()
 {
     PlatformInputContext platformInputContext;