Make sure QGraphicsItem notifies changes to focusScopeItem.

A glitch in QGraphicsItem's logic made it update the focusScopeItem
pointer, but fail to notify the change to QDeclarativeItem through the
d_ptr->focusScopeItemChange() virtual function, hindering
QDeclarativeItem from emitting focusChanged() correctly for focus
scopes that do not have focus.

Two lines were moved, and a comment updated to reflect the reason
why the "return" is needed at this point. It's clear that the
calls to focusScopeItemChange() are unrelated to the return.

Task-number: QTBUG-29260
Change-Id: I12ba9161b16d34c3689401a92c86d2047989f7bd
Reviewed-by: Andreas Aardal Hanssen <andreas@hanssen.name>
Reviewed-by: Jan Arve Sæther <jan-arve.saether@digia.com>
Reviewed-by: Alan Alpert <aalpert@rim.com>

2 files changed, 64 insertions, 5 deletions

diff --git a/src/widgets/graphicsview/qgraphicsitem.cpp b/src/widgets/graphicsview/qgraphicsitem.cpp
index 318c379974..87f5f17531 100644
--- a/src/widgets/graphicsview/qgraphicsitem.cpp
+++ b/src/widgets/graphicsview/qgraphicsitem.cpp
@@ -3237,12 +3237,14 @@ void QGraphicsItemPrivate::setFocusHelper(Qt::FocusReason focusReason, bool clim
         if (p->flags() & QGraphicsItem::ItemIsFocusScope) {
             QGraphicsItem *oldFocusScopeItem = p->d_ptr->focusScopeItem;
             p->d_ptr->focusScopeItem = q_ptr;
+            if (oldFocusScopeItem)
+                oldFocusScopeItem->d_ptr->focusScopeItemChange(false);
+            focusScopeItemChange(true);
             if (!p->focusItem() && !focusFromHide) {
-                if (oldFocusScopeItem)
-                    oldFocusScopeItem->d_ptr->focusScopeItemChange(false);
-                focusScopeItemChange(true);
-                // If you call setFocus on a child of a focus scope that
-                // doesn't currently have a focus item, then stop.
+                // Calling setFocus() on a child of a focus scope that does
+                // not have focus changes only the focus scope pointer,
+                // so that focus is restored the next time the scope gains
+                // focus.
                 return;
             }
             break;
diff --git a/tests/auto/widgets/graphicsview/qgraphicsitem/tst_qgraphicsitem.cpp b/tests/auto/widgets/graphicsview/qgraphicsitem/tst_qgraphicsitem.cpp
index 72014e29b5..b14e68951b 100644
--- a/tests/auto/widgets/graphicsview/qgraphicsitem/tst_qgraphicsitem.cpp
+++ b/tests/auto/widgets/graphicsview/qgraphicsitem/tst_qgraphicsitem.cpp
@@ -428,6 +428,7 @@ private slots:
     void ensureDirtySceneTransform();
     void focusScope();
     void focusScope2();
+    void focusScopeItemChangedWhileScopeDoesntHaveFocus();
     void stackBefore();
     void sceneModality();
     void panelModality();
@@ -9371,6 +9372,62 @@ void tst_QGraphicsItem::focusScope2()
     QCOMPARE(siblingFocusScope->focusItem(), (QGraphicsItem *)siblingChild2);
 }
 
+class FocusScopeItemPrivate;
+class FocusScopeItem : public QGraphicsItem
+{
+    Q_DECLARE_PRIVATE(FocusScopeItem)
+public:
+    FocusScopeItem(QGraphicsItem *parent = 0);
+    QRectF boundingRect() const { return QRectF(); }
+    void paint(QPainter *, const QStyleOptionGraphicsItem *, QWidget *) { }
+
+    int focusScopeChanged;
+    FocusScopeItemPrivate *d_ptr;
+};
+
+class FocusScopeItemPrivate : QGraphicsItemPrivate
+{
+    Q_DECLARE_PUBLIC(FocusScopeItem)
+public:
+    void focusScopeItemChange(bool)
+    { ++q_func()->focusScopeChanged; }
+};
+
+FocusScopeItem::FocusScopeItem(QGraphicsItem *parent)
+    : QGraphicsItem(*new FocusScopeItemPrivate, parent), focusScopeChanged(0)
+{
+    setFlag(ItemIsFocusable);
+}
+
+void tst_QGraphicsItem::focusScopeItemChangedWhileScopeDoesntHaveFocus()
+{
+    QGraphicsRectItem rect;
+    rect.setFlags(QGraphicsItem::ItemIsFocusScope | QGraphicsItem::ItemIsFocusable);
+
+    FocusScopeItem *child1 = new FocusScopeItem(&rect);
+    FocusScopeItem *child2 = new FocusScopeItem(&rect);
+
+    QCOMPARE(rect.focusScopeItem(), (QGraphicsItem *)0);
+    QCOMPARE(child1->focusScopeChanged, 0);
+    QCOMPARE(child2->focusScopeChanged, 0);
+    child1->setFocus();
+    QCOMPARE(rect.focusScopeItem(), (QGraphicsItem *)child1);
+    QCOMPARE(child1->focusScopeChanged, 1);
+    QCOMPARE(child2->focusScopeChanged, 0);
+    child2->setFocus();
+    QCOMPARE(rect.focusScopeItem(), (QGraphicsItem *)child2);
+    QCOMPARE(child1->focusScopeChanged, 2);
+    QCOMPARE(child2->focusScopeChanged, 1);
+    child1->setFocus();
+    QCOMPARE(rect.focusScopeItem(), (QGraphicsItem *)child1);
+    QCOMPARE(child1->focusScopeChanged, 3);
+    QCOMPARE(child2->focusScopeChanged, 2);
+    child1->clearFocus();
+    QCOMPARE(rect.focusScopeItem(), (QGraphicsItem *)0);
+    QCOMPARE(child1->focusScopeChanged, 4);
+    QCOMPARE(child2->focusScopeChanged, 2);
+}
+
 void tst_QGraphicsItem::stackBefore()
 {
     QGraphicsRectItem parent;