Fix dangling pointer issue in QGraphicsItem focus handling

When focused QGraphicsWidget is being hidden, it tries to move the
focus to next focusable widget by calling focusNextPrevChild(true). If
there is no such widget available, the focus is cleared altogether.

The logic used to rely on the return value of focusNextPrevChild to
decide if clearing the focus is necessary. This was incorrect because
the return value may be true also when the focus has actually moved
within the graphics widget in question rather than moving to the next
widget. This is the case at least when GraphicsWebView item is
focused. Thus, the focus could then be left on invisible item which was
a serious issue: elsewhere in the qgraphicsitem.ccp, it is assumed that
no visible item can point to invisible item with its subFocus pointer.
Such pointers will become dangling pointers once the pointed
graphicsitem is deleted, and the process will then crash as soon as the
focus is tried to be moved to another item.

Now, the logic has been modified so that it is explicitely checked
after call to focusNextPrevChild if the item being hidden still has
the focus. If this is the case, the focus is cleared. To be consistent,
the same logic is applied also when disabling a QGraphicsWidget. Having
focus on disabled item is not as severe issue as having it on invisible
item, but it is still wrong.

Task-number: ou1cimx1#995710
Change-Id: Ica32bc381befc3ccaac79fb4cf4d50c5d452fad0
Reviewed-by: Gunnar Sletta <gunnar.sletta@nokia.com>
Reviewed-by: Pasi Pentikäinen <ext-pasi.a.pentikainen@nokia.com>

1 files changed, 6 insertions, 6 deletions

diff --git a/src/gui/graphicsview/qgraphicsitem.cpp b/src/gui/graphicsview/qgraphicsitem.cpp
index d4109ca..66939ac 100644
--- a/src/gui/graphicsview/qgraphicsitem.cpp
+++ b/src/gui/graphicsview/qgraphicsitem.cpp
@@ -2318,16 +2318,16 @@ void QGraphicsItemPrivate::setVisibleHelper(bool newVisible, bool explicitly, bo
         if (hasFocus && scene) {
             // Hiding the closest non-panel ancestor of the focus item
             QGraphicsItem *focusItem = scene->focusItem();
-            bool clear = true;
             if (isWidget && !focusItem->isPanel()) {
                 do {
                     if (focusItem == q_ptr) {
-                        clear = !static_cast<QGraphicsWidget *>(q_ptr)->focusNextPrevChild(true);
+                        static_cast<QGraphicsWidget *>(q_ptr)->focusNextPrevChild(true);
                         break;
                     }
                 } while ((focusItem = focusItem->parentWidget()) && !focusItem->isPanel());
             }
-            if (clear)
+            // Clear focus if previous steps didn't move it to another widget
+            if (q_ptr->hasFocus())
                 clearFocusHelper(/* giveFocusToParent = */ false);
         }
         if (q_ptr->isSelected())
@@ -2506,16 +2506,16 @@ void QGraphicsItemPrivate::setEnabledHelper(bool newEnabled, bool explicitly, bo
             // Disabling the closest non-panel ancestor of the focus item
             // causes focus to pop to the next item, otherwise it's cleared.
             QGraphicsItem *focusItem = scene->focusItem();
-            bool clear = true;
             if (isWidget && !focusItem->isPanel() && q_ptr->isAncestorOf(focusItem)) {
                 do {
                     if (focusItem == q_ptr) {
-                        clear = !static_cast<QGraphicsWidget *>(q_ptr)->focusNextPrevChild(true);
+                        static_cast<QGraphicsWidget *>(q_ptr)->focusNextPrevChild(true);
                         break;
                     }
                 } while ((focusItem = focusItem->parentWidget()) && !focusItem->isPanel());
             }
-            if (clear)
+            // Clear focus if previous steps didn't move it to another widget
+            if (q_ptr->hasFocus())
                 q_ptr->clearFocus();
         }
         if (q_ptr->isSelected())