Fix crash/bug in QGraphicsItem's subFocus handling.

Removes dangling subFocusItem pointers when changing focus after
reparenting. This change also includes a mini-optimization when adding
focusable items to an inactive scene.

Reviewed-by: brad

3 files changed, 25 insertions, 8 deletions

diff --git a/src/gui/graphicsview/qgraphicsitem.cpp b/src/gui/graphicsview/qgraphicsitem.cpp
index e553517..9c0c649 100644
--- a/src/gui/graphicsview/qgraphicsitem.cpp
+++ b/src/gui/graphicsview/qgraphicsitem.cpp
@@ -1198,6 +1198,7 @@ QGraphicsItem::~QGraphicsItem()
     d_ptr->removeExtraItemCache();
 
     clearFocus();
+
     if (!d_ptr->children.isEmpty()) {
         QList<QGraphicsItem *> oldChildren = d_ptr->children;
         qDeleteAll(oldChildren);
@@ -2750,7 +2751,7 @@ void QGraphicsItem::setFocus(Qt::FocusReason focusReason)
     // Update the scene's focus item.
     if (d_ptr->scene) {
         QGraphicsItem *p = panel();
-        if ((!p && d_ptr->scene->isActive()) || p->isActive()) {
+        if ((!p && d_ptr->scene->isActive()) || (p && p->isActive())) {
             // Visible items immediately gain focus from scene.
             d_ptr->scene->d_func()->setFocusItemHelper(f, focusReason);
         }
@@ -2770,10 +2771,9 @@ void QGraphicsItem::setFocus(Qt::FocusReason focusReason)
 */
 void QGraphicsItem::clearFocus()
 {
-    if (!d_ptr->scene)
-        return;
     // Invisible items with focus must explicitly clear subfocus.
     d_ptr->clearSubFocus();
+
     if (hasFocus()) {
         // If this item has the scene's input focus, clear it.
         d_ptr->scene->setFocusItem(0);
@@ -4856,10 +4856,15 @@ void QGraphicsItemPrivate::setSubFocus()
 {
     // Update focus child chain. Stop at panels, or if this item
     // is hidden, stop at the first item with a visible parent.
-    QGraphicsItem *item = q_ptr;
-    QGraphicsItem *parent = item;
+    QGraphicsItem *parent = q_ptr;
     do {
-        parent->d_func()->subFocusItem = item;
+        // Clear any existing ancestor's subFocusItem.
+        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 = q_ptr;
     } while (!parent->isPanel() && (parent = parent->d_ptr->parent) && (visible || !parent->d_ptr->visible));
 
     if (!parent && scene && !scene->isActive())
@@ -4871,7 +4876,7 @@ void QGraphicsItemPrivate::setSubFocus()
 */
 void QGraphicsItemPrivate::clearSubFocus()
 {
-    // Reset focus child chain.
+    // Reset sub focus chain.
     QGraphicsItem *parent = q_ptr;
     do {
         if (parent->d_ptr->subFocusItem != q_ptr)
diff --git a/src/gui/graphicsview/qgraphicsscene.cpp b/src/gui/graphicsview/qgraphicsscene.cpp
index 43f2932..0fd1647 100644
--- a/src/gui/graphicsview/qgraphicsscene.cpp
+++ b/src/gui/graphicsview/qgraphicsscene.cpp
@@ -1390,6 +1390,7 @@ QGraphicsScene::QGraphicsScene(qreal x, qreal y, qreal width, qreal height, QObj
 QGraphicsScene::~QGraphicsScene()
 {
     Q_D(QGraphicsScene);
+
     // Remove this scene from qApp's global scene list.
     qApp->d_func()->scene_list.removeAll(this);
 
@@ -2430,7 +2431,7 @@ void QGraphicsScene::addItem(QGraphicsItem *item)
 
     // Ensure that newly added items that have subfocus set, gain
     // focus automatically if there isn't a focus item already.
-    if (!d->focusItem && item->focusItem())
+    if (!d->focusItem && item->focusItem() && item->isActive())
         item->focusItem()->setFocus();
 
     d->updateInputMethodSensitivityInViews();
diff --git a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp
index 65837ae..5e8f4c4 100644
--- a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp
+++ b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp
@@ -7620,6 +7620,7 @@ void tst_QGraphicsItem::subFocus()
     QCOMPARE(text2->focusItem(), (QGraphicsItem *)text2);
     text2->setParentItem(text);
     QCOMPARE(text->focusItem(), (QGraphicsItem *)text2);
+    QCOMPARE(text2->focusItem(), (QGraphicsItem *)text2);
     QVERIFY(!text->hasFocus());
     QVERIFY(text2->hasFocus());
 
@@ -7644,6 +7645,12 @@ void tst_QGraphicsItem::subFocus()
     QGraphicsRectItem *rect3 = new QGraphicsRectItem(rect2);
     rect3->setFlag(QGraphicsItem::ItemIsFocusable);
 
+    text->setData(0, "text");
+    text2->setData(0, "text2");
+    rect->setData(0, "rect");
+    rect2->setData(0, "rect2");
+    rect3->setData(0, "rect3");
+
     rect3->setFocus();
     QVERIFY(!rect3->hasFocus());
     QCOMPARE(rect->focusItem(), (QGraphicsItem *)rect3);
@@ -7651,6 +7658,10 @@ void tst_QGraphicsItem::subFocus()
     QCOMPARE(rect3->focusItem(), (QGraphicsItem *)rect3);
     rect->setParentItem(text2);
     QCOMPARE(text->focusItem(), (QGraphicsItem *)rect3);
+    QCOMPARE(text2->focusItem(), (QGraphicsItem *)rect3);
+    QCOMPARE(rect->focusItem(), (QGraphicsItem *)rect3);
+    QCOMPARE(rect2->focusItem(), (QGraphicsItem *)rect3);
+    QCOMPARE(rect3->focusItem(), (QGraphicsItem *)rect3);
     QVERIFY(!rect->hasFocus());
     QVERIFY(!rect2->hasFocus());
     QVERIFY(rect3->hasFocus());