Don't emit activeFocusChanged() unless the active focus actually changed

We would previously call subFocusItemChanged(0) on the item as part of
clearing the subfocus, even if the item in question would recieve a new
subfocus item as part of setting the new subfocus.

This resulted in the declarative item emitting activeFocusChanged(false)
and then activeFocusChanged(true), which was affecting any animation or
state bound to the activeFocus property of the item.

We now stop clearing the subfocus when encountering an item that we know
will get subfocus during the set-subfocus pass. We then set subfocus all
the way to the root item, since the subfocus item itself might change.

The effect of this is that the declarative item will only get one call
to subFocusItemChanged(), passing the new subfocus item, instead of two.
This means the declarative item can keep track of wherther ot not it had
a subfocus item previously, and only emit activeFocusChanged() when the
active focus goes from true to false or false to true.

Task-number: QTBUG-15615
Reviewed-by: Yoann Lopes <yoann.lopes@nokia.com>

5 files changed, 155 insertions, 12 deletions

diff --git a/src/declarative/graphicsitems/qdeclarativeitem_p.h b/src/declarative/graphicsitems/qdeclarativeitem_p.h
index d8635b9..a36ee34 100644
--- a/src/declarative/graphicsitems/qdeclarativeitem_p.h
+++ b/src/declarative/graphicsitems/qdeclarativeitem_p.h
@@ -126,7 +126,7 @@ public:
       widthValid(false), heightValid(false),
       componentComplete(true), keepMouse(false),
       smooth(false), transformOriginDirty(true), doneEventPreHandler(false), keyHandler(0),
-      mWidth(0), mHeight(0), implicitWidth(0), implicitHeight(0)
+      mWidth(0), mHeight(0), implicitWidth(0), implicitHeight(0), hadSubFocusItem(false)
     {
         QGraphicsItemPrivate::acceptedMouseButtons = 0;
         isDeclarativeItem = 1;
@@ -275,6 +275,8 @@ public:
     qreal implicitWidth;
     qreal implicitHeight;
 
+    bool hadSubFocusItem;
+
     QPointF computeTransformOrigin() const;
 
     virtual void setPosHelper(const QPointF &pos)
@@ -288,9 +290,11 @@ public:
     // Reimplemented from QGraphicsItemPrivate
     virtual void subFocusItemChange()
     {
-        if (flags & QGraphicsItem::ItemIsFocusScope || !parent)
-            emit q_func()->activeFocusChanged(subFocusItem != 0);
+        bool hasSubFocusItem = subFocusItem != 0;
+        if (((flags & QGraphicsItem::ItemIsFocusScope) || !parent) && hasSubFocusItem != hadSubFocusItem)
+            emit q_func()->activeFocusChanged(hasSubFocusItem);
         //see also QDeclarativeItemPrivate::focusChanged
+        hadSubFocusItem = hasSubFocusItem;
     }
 
     // Reimplemented from QGraphicsItemPrivate
diff --git a/src/gui/graphicsview/qgraphicsitem.cpp b/src/gui/graphicsview/qgraphicsitem.cpp
index 7ab49cb..94e1a72 100644
--- a/src/gui/graphicsview/qgraphicsitem.cpp
+++ b/src/gui/graphicsview/qgraphicsitem.cpp
@@ -3295,9 +3295,13 @@ void QGraphicsItemPrivate::setFocusHelper(Qt::FocusReason focusReason, bool clim
     }
 
     // Update the child focus chain.
-    if (scene && scene->focusItem())
-        scene->focusItem()->d_ptr->clearSubFocus();
-    f->d_ptr->setSubFocus();
+    QGraphicsItem *commonAncestor = 0;
+    if (scene && scene->focusItem()) {
+        commonAncestor = scene->focusItem()->commonAncestorItem(f);
+        scene->focusItem()->d_ptr->clearSubFocus(scene->focusItem(), commonAncestor);
+    }
+
+    f->d_ptr->setSubFocus(f, commonAncestor);
 
     // Update the scene's focus item.
     if (scene) {
@@ -5554,7 +5558,7 @@ void QGraphicsItemPrivate::ensureSceneTransformRecursive(QGraphicsItem **topMost
 /*!
     \internal
 */
-void QGraphicsItemPrivate::setSubFocus(QGraphicsItem *rootItem)
+void QGraphicsItemPrivate::setSubFocus(QGraphicsItem *rootItem, QGraphicsItem *stopItem)
 {
     // Update focus child chain. Stop at panels, or if this item
     // is hidden, stop at the first item with a visible parent.
@@ -5567,7 +5571,7 @@ void QGraphicsItemPrivate::setSubFocus(QGraphicsItem *rootItem)
         if (parent != q_ptr && parent->d_ptr->subFocusItem) {
             if (parent->d_ptr->subFocusItem == q_ptr)
                 break;
-            parent->d_ptr->subFocusItem->d_ptr->clearSubFocus();
+            parent->d_ptr->subFocusItem->d_ptr->clearSubFocus(0, stopItem);
         }
         parent->d_ptr->subFocusItem = q_ptr;
         parent->d_ptr->subFocusItemChange();
@@ -5580,12 +5584,12 @@ void QGraphicsItemPrivate::setSubFocus(QGraphicsItem *rootItem)
 /*!
     \internal
 */
-void QGraphicsItemPrivate::clearSubFocus(QGraphicsItem *rootItem)
+void QGraphicsItemPrivate::clearSubFocus(QGraphicsItem *rootItem, QGraphicsItem *stopItem)
 {
     // Reset sub focus chain.
     QGraphicsItem *parent = rootItem ? rootItem : q_ptr;
     do {
-        if (parent->d_ptr->subFocusItem != q_ptr)
+        if (parent->d_ptr->subFocusItem != q_ptr || parent == stopItem)
             break;
         parent->d_ptr->subFocusItem = 0;
         parent->d_ptr->subFocusItemChange();
diff --git a/src/gui/graphicsview/qgraphicsitem_p.h b/src/gui/graphicsview/qgraphicsitem_p.h
index 1b7aa97..b938759 100644
--- a/src/gui/graphicsview/qgraphicsitem_p.h
+++ b/src/gui/graphicsview/qgraphicsitem_p.h
@@ -479,8 +479,8 @@ public:
 
     void setFocusHelper(Qt::FocusReason focusReason, bool climb, bool focusFromHide);
     void clearFocusHelper(bool giveFocusToParent);
-    void setSubFocus(QGraphicsItem *rootItem = 0);
-    void clearSubFocus(QGraphicsItem *rootItem = 0);
+    void setSubFocus(QGraphicsItem *rootItem = 0, QGraphicsItem *stopItem = 0);
+    void clearSubFocus(QGraphicsItem *rootItem = 0, QGraphicsItem *stopItem = 0);
     void resetFocusProxy();
     virtual void subFocusItemChange();
     virtual void focusScopeItemChange(bool isSubFocusItem);
diff --git a/tests/auto/declarative/qdeclarativefocusscope/data/forceActiveFocus.qml b/tests/auto/declarative/qdeclarativefocusscope/data/forceActiveFocus.qml
new file mode 100644
index 0000000..6c39d4a
--- /dev/null
+++ b/tests/auto/declarative/qdeclarativefocusscope/data/forceActiveFocus.qml
@@ -0,0 +1,26 @@
+import QtQuick 1.0
+
+Rectangle {
+    objectName: "root"
+    FocusScope {
+        objectName: "scope"
+        Item {
+            objectName: "item-a1"
+            FocusScope {
+                objectName: "scope-a"
+                Item {
+                    objectName: "item-a2"
+                }
+            }
+        }
+        Item {
+            objectName: "item-b1"
+            FocusScope {
+                objectName: "scope-b"
+                Item {
+                    objectName: "item-b2"
+                }
+            }
+        }
+    }
+}
diff --git a/tests/auto/declarative/qdeclarativefocusscope/tst_qdeclarativefocusscope.cpp b/tests/auto/declarative/qdeclarativefocusscope/tst_qdeclarativefocusscope.cpp
index 1645dac..6a3627b 100644
--- a/tests/auto/declarative/qdeclarativefocusscope/tst_qdeclarativefocusscope.cpp
+++ b/tests/auto/declarative/qdeclarativefocusscope/tst_qdeclarativefocusscope.cpp
@@ -71,6 +71,7 @@ private slots:
     void noParentFocus();
     void signalEmission();
     void qtBug13380();
+    void forceActiveFocus();
 };
 
 /*
@@ -432,6 +433,114 @@ void tst_qdeclarativefocusscope::qtBug13380()
     delete view;
 }
 
+void tst_qdeclarativefocusscope::forceActiveFocus()
+{
+    QDeclarativeView *view = new QDeclarativeView;
+    view->setSource(QUrl::fromLocalFile(SRCDIR "/data/forceActiveFocus.qml"));
+
+    QGraphicsObject *rootObject = view->rootObject();
+    QVERIFY(rootObject);
+
+    QDeclarativeItem *scope = findItem<QDeclarativeItem>(rootObject, QLatin1String("scope"));
+    QDeclarativeItem *itemA1 = findItem<QDeclarativeItem>(rootObject, QLatin1String("item-a1"));
+    QDeclarativeItem *scopeA = findItem<QDeclarativeItem>(rootObject, QLatin1String("scope-a"));
+    QDeclarativeItem *itemA2 = findItem<QDeclarativeItem>(rootObject, QLatin1String("item-a2"));
+    QDeclarativeItem *itemB1 = findItem<QDeclarativeItem>(rootObject, QLatin1String("item-b1"));
+    QDeclarativeItem *scopeB = findItem<QDeclarativeItem>(rootObject, QLatin1String("scope-b"));
+    QDeclarativeItem *itemB2 = findItem<QDeclarativeItem>(rootObject, QLatin1String("item-b2"));
+
+    QVERIFY(scope);
+    QVERIFY(itemA1);
+    QVERIFY(scopeA);
+    QVERIFY(itemA2);
+    QVERIFY(itemB1);
+    QVERIFY(scopeB);
+    QVERIFY(itemB2);
+
+    QSignalSpy rootSpy(rootObject, SIGNAL(activeFocusChanged(bool)));
+    QSignalSpy scopeSpy(scope, SIGNAL(activeFocusChanged(bool)));
+    QSignalSpy scopeASpy(scopeA, SIGNAL(activeFocusChanged(bool)));
+    QSignalSpy scopeBSpy(scopeB, SIGNAL(activeFocusChanged(bool)));
+
+    // First, walk the focus from item-a1 down to item-a2 and back again
+    itemA1->forceActiveFocus();
+    QVERIFY(itemA1->hasActiveFocus());
+    QCOMPARE(rootSpy.count(), 1);
+    QCOMPARE(scopeSpy.count(), 1);
+
+    scopeA->forceActiveFocus();
+    QVERIFY(!itemA1->hasActiveFocus());
+    QVERIFY(scopeA->hasActiveFocus());
+    QCOMPARE(scopeASpy.count(), 1);
+    QCOMPARE(rootSpy.count(), 1);
+    QCOMPARE(scopeSpy.count(), 1);
+
+    itemA2->forceActiveFocus();
+    QVERIFY(!itemA1->hasActiveFocus());
+    QVERIFY(itemA2->hasActiveFocus());
+    QVERIFY(scopeA->hasActiveFocus());
+    QCOMPARE(scopeASpy.count(), 1);
+    QCOMPARE(rootSpy.count(), 1);
+    QCOMPARE(scopeSpy.count(), 1);
+
+    scopeA->forceActiveFocus();
+    QVERIFY(!itemA1->hasActiveFocus());
+    QVERIFY(itemA2->hasActiveFocus());
+    QVERIFY(scopeA->hasActiveFocus());
+    QCOMPARE(scopeASpy.count(), 1);
+    QCOMPARE(rootSpy.count(), 1);
+    QCOMPARE(scopeSpy.count(), 1);
+
+    itemA1->forceActiveFocus();
+    QVERIFY(itemA1->hasActiveFocus());
+    QVERIFY(!scopeA->hasActiveFocus());
+    QVERIFY(!itemA2->hasActiveFocus());
+    QCOMPARE(scopeASpy.count(), 2);
+    QCOMPARE(rootSpy.count(), 1);
+    QCOMPARE(scopeSpy.count(), 1);
+
+    // Then jump back and forth between branch 'a' and 'b'
+    itemB1->forceActiveFocus();
+    QVERIFY(itemB1->hasActiveFocus());
+    QCOMPARE(rootSpy.count(), 1);
+    QCOMPARE(scopeSpy.count(), 1);
+
+    scopeA->forceActiveFocus();
+    QVERIFY(!itemA1->hasActiveFocus());
+    QVERIFY(!itemB1->hasActiveFocus());
+    QVERIFY(scopeA->hasActiveFocus());
+    QCOMPARE(scopeASpy.count(), 3);
+    QCOMPARE(rootSpy.count(), 1);
+    QCOMPARE(scopeSpy.count(), 1);
+
+    scopeB->forceActiveFocus();
+    QVERIFY(!scopeA->hasActiveFocus());
+    QVERIFY(!itemB1->hasActiveFocus());
+    QVERIFY(scopeB->hasActiveFocus());
+    QCOMPARE(scopeASpy.count(), 4);
+    QCOMPARE(scopeBSpy.count(), 1);
+    QCOMPARE(rootSpy.count(), 1);
+    QCOMPARE(scopeSpy.count(), 1);
+
+    itemA2->forceActiveFocus();
+    QVERIFY(!scopeB->hasActiveFocus());
+    QVERIFY(itemA2->hasActiveFocus());
+    QCOMPARE(scopeASpy.count(), 5);
+    QCOMPARE(scopeBSpy.count(), 2);
+    QCOMPARE(rootSpy.count(), 1);
+    QCOMPARE(scopeSpy.count(), 1);
+
+    itemB2->forceActiveFocus();
+    QVERIFY(!itemA2->hasActiveFocus());
+    QVERIFY(itemB2->hasActiveFocus());
+    QCOMPARE(scopeASpy.count(), 6);
+    QCOMPARE(scopeBSpy.count(), 3);
+    QCOMPARE(rootSpy.count(), 1);
+    QCOMPARE(scopeSpy.count(), 1);
+
+    delete view;
+}
+
 QTEST_MAIN(tst_qdeclarativefocusscope)
 
 #include "tst_qdeclarativefocusscope.moc"