FocusScope's focusItem must always be a descendent

Task-number: QTBUG-20699
Reviewed-by: Michael Brasser

2 files changed, 50 insertions, 16 deletions

diff --git a/src/gui/graphicsview/qgraphicsitem.cpp b/src/gui/graphicsview/qgraphicsitem.cpp
index 2ac2bdf..0c218fc 100644
--- a/src/gui/graphicsview/qgraphicsitem.cpp
+++ b/src/gui/graphicsview/qgraphicsitem.cpp
@@ -1171,24 +1171,26 @@ void QGraphicsItemPrivate::setParentItemHelper(QGraphicsItem *newParent, const Q
     // Update focus scope item ptr in new scope.
     QGraphicsItem *newFocusScopeItem = subFocusItem ? subFocusItem : parentFocusScopeItem;
     if (newFocusScopeItem && newParent) {
-        if (subFocusItem) {
-            // Find the subFocusItem's topmost focus scope.
-            QGraphicsItem *ancestorScope = 0;
-            QGraphicsItem *p = subFocusItem->d_ptr->parent;
-            while (p) {
-                if (p->d_ptr->flags & QGraphicsItem::ItemIsFocusScope)
-                    ancestorScope = p;
-                if (p->d_ptr->flags & QGraphicsItem::ItemIsPanel)
-                    break;
-                p = p->d_ptr->parent;
-            }
-            if (ancestorScope)
-                newFocusScopeItem = ancestorScope;
-        }
-
         QGraphicsItem *p = newParent;
         while (p) {
             if (p->d_ptr->flags & QGraphicsItem::ItemIsFocusScope) {
+                if (subFocusItem && subFocusItem != q_ptr) {
+                    // Find the subFocusItem's topmost focus scope within the new parent's focusscope
+                    QGraphicsItem *ancestorScope = 0;
+                    QGraphicsItem *p2 = subFocusItem->d_ptr->parent;
+                    while (p2 && p2 != p) {
+                        if (p2->d_ptr->flags & QGraphicsItem::ItemIsFocusScope)
+                            ancestorScope = p2;
+                        if (p2->d_ptr->flags & QGraphicsItem::ItemIsPanel)
+                            break;
+                        if (p2 == q_ptr)
+                            break;
+                        p2 = p2->d_ptr->parent;
+                    }
+                    if (ancestorScope)
+                        newFocusScopeItem = ancestorScope;
+                }
+
                 p->d_ptr->focusScopeItem = newFocusScopeItem;
                 newFocusScopeItem->d_ptr->focusScopeItemChange(true);
                 // Ensure the new item is no longer the subFocusItem. The
@@ -3297,7 +3299,7 @@ void QGraphicsItemPrivate::setFocusHelper(Qt::FocusReason focusReason, bool clim
     }
 
     if (climb) {
-        while (f->d_ptr->focusScopeItem && f->d_ptr->focusScopeItem->isVisible() && f->d_ptr->focusScopeItem != f)
+        while (f->d_ptr->focusScopeItem && f->d_ptr->focusScopeItem->isVisible())
             f = f->d_ptr->focusScopeItem;
     }
 
diff --git a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp
index 3665b10..9b834d5 100644
--- a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp
+++ b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp
@@ -475,6 +475,7 @@ private slots:
     void QTBUG_12112_focusItem();
     void QTBUG_13473_sceneposchange();
     void QTBUG_16374_crashInDestructor();
+    void QTBUG_20699_focusScopeCrash();
 
 private:
     QList<QGraphicsItem *> paintedItems;
@@ -11265,5 +11266,36 @@ void tst_QGraphicsItem::QTBUG_16374_crashInDestructor()
     QTest::qWaitForWindowShown(&view);
 }
 
+void tst_QGraphicsItem::QTBUG_20699_focusScopeCrash()
+{
+    QGraphicsScene scene;
+    QGraphicsView view(&scene);
+    QGraphicsPixmapItem fs;
+    fs.setFlags(QGraphicsItem::ItemIsFocusScope | QGraphicsItem::ItemIsFocusable);
+    scene.addItem(&fs);
+    QGraphicsPixmapItem* fs2 = new QGraphicsPixmapItem(&fs);
+    fs2->setFlags(QGraphicsItem::ItemIsFocusScope | QGraphicsItem::ItemIsFocusable);
+    QGraphicsPixmapItem* fi2 = new QGraphicsPixmapItem(&fs);
+    fi2->setFlags(QGraphicsItem::ItemIsFocusable);
+    QGraphicsPixmapItem* fi = new QGraphicsPixmapItem(fs2);
+    fi->setFlags(QGraphicsItem::ItemIsFocusable);
+    fs.setFocus();
+    fi->setFocus();
+
+    view.show();
+    QTest::qWaitForWindowShown(&view);
+
+    fi->setParentItem(fi2);
+    fi->setFocus();
+    fs.setFocus();
+    fi->setParentItem(fs2);
+    fi->setFocus();
+    fs2->setFocus();
+    fs.setFocus();
+    fi->setParentItem(fi2);
+    fi->setFocus();
+    fs.setFocus();
+}
+
 QTEST_MAIN(tst_QGraphicsItem)
 #include "tst_qgraphicsitem.moc"