From cf6d24bee40dfff2711990dc61e04aa8193d89cf Mon Sep 17 00:00:00 2001 From: Pauli Jarvinen Date: Wed, 30 May 2012 13:09:46 +0300 Subject: Fix dangling pointer issue in QGraphicsItem focus handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Pasi Pentikäinen --- src/gui/graphicsview/qgraphicsitem.cpp | 12 ++++++------ 1 file 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(q_ptr)->focusNextPrevChild(true); + static_cast(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(q_ptr)->focusNextPrevChild(true); + static_cast(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()) -- cgit v0.12