From ebb1162f54a29baeccb71d1e283146892629518f Mon Sep 17 00:00:00 2001 From: Andreas Aardal Hanssen Date: Wed, 9 Sep 2009 17:10:58 +0200 Subject: 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 --- src/gui/graphicsview/qgraphicsitem.cpp | 19 ++++++++++++------- src/gui/graphicsview/qgraphicsscene.cpp | 3 ++- tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp | 11 +++++++++++ 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 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()); -- cgit v0.12