Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Update item focus even if it doesn't have a canvas
Currently the item focus data is not updated if it is not in a canvas.
This means a subFocusItem may be deleted when the item is outside of a
canvas, creating a stale pointer when the item is moved back into a
canvas.

This change also means that the last item to set focus=true will now
consistently get activeFocus. Previously if an item did not have a
canvas and then was moved back into the canvas, the first item found
with focus=true would get activeFocus.

Task-number: QTBUG-24616
Change-Id: Ia706bd6ba6bcbccd616b5019c7c0fae4c39afa7f
Reviewed-by: Andrew den Exter <andrew.den-exter@nokia.com>
  • Loading branch information
Bea Lam authored and Qt by Nokia committed Mar 20, 2012
1 parent d96d89c commit d268ac6
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 56 deletions.
2 changes: 1 addition & 1 deletion src/quick/items/qquickcanvas_p.h
Expand Up @@ -144,7 +144,7 @@ class Q_QUICK_EXPORT QQuickCanvasPrivate : public QWindowPrivate

void setFocusInScope(QQuickItem *scope, QQuickItem *item, FocusOptions = 0);
void clearFocusInScope(QQuickItem *scope, QQuickItem *item, FocusOptions = 0);
void notifyFocusChangesRecur(QQuickItem **item, int remaining);
static void notifyFocusChangesRecur(QQuickItem **item, int remaining);

void updateFocusItemTransform();

Expand Down
89 changes: 57 additions & 32 deletions src/quick/items/qquickitem.cpp
Expand Up @@ -1924,17 +1924,21 @@ void QQuickItem::setParentItem(QQuickItem *parentItem)

QQuickItem *scopeItem = 0;

if (d->canvas && hasFocus()) {
if (hasFocus())
scopeFocusedItem = this;
} else if (d->canvas && !isFocusScope() && d->subFocusItem) {
else if (!isFocusScope() && d->subFocusItem)
scopeFocusedItem = d->subFocusItem;
}

if (scopeFocusedItem) {
scopeItem = oldParentItem;
while (!scopeItem->isFocusScope()) scopeItem = scopeItem->parentItem();
QQuickCanvasPrivate::get(d->canvas)->clearFocusInScope(scopeItem, scopeFocusedItem,
while (!scopeItem->isFocusScope() && scopeItem->parentItem())
scopeItem = scopeItem->parentItem();
if (d->canvas) {
QQuickCanvasPrivate::get(d->canvas)->clearFocusInScope(scopeItem, scopeFocusedItem,
QQuickCanvasPrivate::DontChangeFocusProperty);
} else {
QQuickItemPrivate::get(scopeFocusedItem)->updateSubFocusItem(scopeItem, false);
}
}

const bool wasVisible = isVisible();
Expand Down Expand Up @@ -1963,18 +1967,31 @@ void QQuickItem::setParentItem(QQuickItem *parentItem)
d->setEffectiveVisibleRecur(d->calcEffectiveVisible());
d->setEffectiveEnableRecur(0, d->calcEffectiveEnable());

if (scopeFocusedItem && d->parentItem && d->canvas) {
// We need to test whether this item becomes scope focused
QQuickItem *scopeItem = 0;
scopeItem = d->parentItem;
while (!scopeItem->isFocusScope()) scopeItem = scopeItem->parentItem();
if (d->parentItem) {
if (!scopeFocusedItem) {
if (hasFocus())
scopeFocusedItem = this;
else if (!isFocusScope() && d->subFocusItem)
scopeFocusedItem = d->subFocusItem;
}

if (scopeItem->scopedFocusItem()) {
QQuickItemPrivate::get(scopeFocusedItem)->focus = false;
emit scopeFocusedItem->focusChanged(false);
} else {
QQuickCanvasPrivate::get(d->canvas)->setFocusInScope(scopeItem, scopeFocusedItem,
QQuickCanvasPrivate::DontChangeFocusProperty);
if (scopeFocusedItem) {
// We need to test whether this item becomes scope focused
QQuickItem *scopeItem = d->parentItem;
while (!scopeItem->isFocusScope() && scopeItem->parentItem())
scopeItem = scopeItem->parentItem();

if (scopeItem->scopedFocusItem()) {
QQuickItemPrivate::get(scopeFocusedItem)->focus = false;
emit scopeFocusedItem->focusChanged(false);
} else {
if (d->canvas) {
QQuickCanvasPrivate::get(d->canvas)->setFocusInScope(scopeItem, scopeFocusedItem,
QQuickCanvasPrivate::DontChangeFocusProperty);
} else {
QQuickItemPrivate::get(scopeFocusedItem)->updateSubFocusItem(scopeItem, true);
}
}
}
}

Expand Down Expand Up @@ -2210,16 +2227,6 @@ void QQuickItemPrivate::initCanvas(InitializationState *state, QQuickCanvas *c)
QQuickItemPrivate::get(child)->initCanvas(childState, c);
}

if (c && focus) {
// Fixup
if (state->getFocusScope(q)->scopedFocusItem()) {
focus = false;
emit q->focusChanged(false);
} else {
QQuickCanvasPrivate::get(canvas)->setFocusInScope(state->getFocusScope(q), q);
}
}

dirty(Canvas);

if (extra.isAllocated() && extra->screenAttached)
Expand Down Expand Up @@ -4672,15 +4679,33 @@ void QQuickItem::setFocus(bool focus)
if (d->focus == focus)
return;

if (d->canvas) {
if (d->canvas || d->parentItem) {
// Need to find our nearest focus scope
QQuickItem *scope = parentItem();
while (scope && !scope->isFocusScope())
while (scope && !scope->isFocusScope() && scope->parentItem())
scope = scope->parentItem();
if (focus)
QQuickCanvasPrivate::get(d->canvas)->setFocusInScope(scope, this);
else
QQuickCanvasPrivate::get(d->canvas)->clearFocusInScope(scope, this);
if (d->canvas) {
if (focus)
QQuickCanvasPrivate::get(d->canvas)->setFocusInScope(scope, this);
else
QQuickCanvasPrivate::get(d->canvas)->clearFocusInScope(scope, this);
} else {
// do the focus changes from setFocusInScope/clearFocusInScope that are
// unrelated to a canvas
QVarLengthArray<QQuickItem *, 20> changed;
QQuickItem *oldSubFocusItem = QQuickItemPrivate::get(scope)->subFocusItem;
if (oldSubFocusItem) {
QQuickItemPrivate::get(oldSubFocusItem)->focus = false;
changed << oldSubFocusItem;
}
d->updateSubFocusItem(scope, focus);

d->focus = focus;
changed << this;
emit focusChanged(focus);

QQuickCanvasPrivate::notifyFocusChangesRecur(changed.data(), changed.count() - 1);
}
} else {
d->focus = focus;
emit focusChanged(focus);
Expand Down
80 changes: 57 additions & 23 deletions tests/auto/quick/qquickitem/tst_qquickitem.cpp
Expand Up @@ -181,7 +181,7 @@ void tst_qquickitem::initTestCase()
qmlRegisterType<TestPolishItem>("Qt.test", 1, 0, "TestPolishItem");
}

// Focus has no effect when outside a canvas
// Focus still updates when outside a canvas
void tst_qquickitem::noCanvas()
{
QQuickItem *root = new TestItem;
Expand All @@ -201,7 +201,7 @@ void tst_qquickitem::noCanvas()
scopedChild2->setFocus(true);
QCOMPARE(root->hasFocus(), true);
QCOMPARE(child->hasFocus(), false);
QCOMPARE(scope->hasFocus(), true);
QCOMPARE(scope->hasFocus(), false);
QCOMPARE(scopedChild->hasFocus(), false);
QCOMPARE(scopedChild2->hasFocus(), true);

Expand All @@ -210,10 +210,10 @@ void tst_qquickitem::noCanvas()
scopedChild->setFocus(true);
scope->setFocus(false);
QCOMPARE(root->hasFocus(), false);
QCOMPARE(child->hasFocus(), true);
QCOMPARE(child->hasFocus(), false);
QCOMPARE(scope->hasFocus(), false);
QCOMPARE(scopedChild->hasFocus(), true);
QCOMPARE(scopedChild2->hasFocus(), true);
QCOMPARE(scopedChild2->hasFocus(), false);

delete root;
}
Expand Down Expand Up @@ -434,7 +434,7 @@ void tst_qquickitem::addedToCanvas()
c1->setFocus(true);
c2->setFocus(true);
focusState[item].set(true, true);
focusState[c1].set(true, false);
focusState[c1].set(false, false);
focusState[c2].set(true, false);
focusState.active(item);
FVERIFY();
Expand All @@ -458,14 +458,14 @@ void tst_qquickitem::addedToCanvas()
focusState << tree << c1 << c2;
c1->setFocus(true);
c2->setFocus(true);
focusState[c1].set(true, false);
focusState[c1].set(false, false);
focusState[c2].set(true, false);
FVERIFY();

tree->setParentItem(canvas.rootItem());
focusState[c1].set(true, true);
focusState[c2].set(false, false);
focusState.active(c1);
focusState[c1].set(false, false);
focusState[c2].set(true, true);
focusState.active(c2);
FVERIFY();
}

Expand All @@ -481,19 +481,19 @@ void tst_qquickitem::addedToCanvas()
focusState << tree << c1 << c2;
c1->setFocus(true);
c2->setFocus(true);
focusState[c1].set(true, false);
focusState[c1].set(false, false);
focusState[c2].set(true, false);
FVERIFY();

tree->setParentItem(canvas.rootItem());
focusState[c1].set(true, false);
focusState[c2].set(false, false);
focusState[c1].set(false, false);
focusState[c2].set(true, false);
FVERIFY();

tree->setFocus(true);
focusState[tree].set(true, true);
focusState[c1].set(true, true);
focusState.active(c1);
focusState[c2].set(true, true);
focusState.active(c2);
FVERIFY();
}

Expand All @@ -511,15 +511,15 @@ void tst_qquickitem::addedToCanvas()
c1->setFocus(true);
c2->setFocus(true);
focusState[tree].set(true, false);
focusState[c1].set(true, false);
focusState[c1].set(false, false);
focusState[c2].set(true, false);
FVERIFY();

tree->setParentItem(canvas.rootItem());
focusState[tree].set(true, true);
focusState[c1].set(true, true);
focusState[c2].set(false, false);
focusState.active(c1);
focusState[c1].set(false, false);
focusState[c2].set(true, true);
focusState.active(c2);
FVERIFY();
}

Expand All @@ -540,22 +540,22 @@ void tst_qquickitem::addedToCanvas()
c2->setFocus(true);
focusState[child].set(true, true);
focusState[tree].set(true, false);
focusState[c1].set(true, false);
focusState[c1].set(false, false);
focusState[c2].set(true, false);
focusState.active(child);
FVERIFY();

tree->setParentItem(canvas.rootItem());
focusState[tree].set(false, false);
focusState[c1].set(true, false);
focusState[c2].set(false, false);
focusState[c1].set(false, false);
focusState[c2].set(true, false);
FVERIFY();

tree->setFocus(true);
focusState[child].set(false, false);
focusState[tree].set(true, true);
focusState[c1].set(true, true);
focusState.active(c1);
focusState[c2].set(true, true);
focusState.active(c2);
FVERIFY();
}
}
Expand Down Expand Up @@ -674,6 +674,40 @@ void tst_qquickitem::changeParent()
FVERIFY();
}

// child has active focus, then its fs parent changes parent to 0, then
// child is deleted, then its parent changes again to a valid parent
{
QQuickCanvas canvas;
ensureFocus(&canvas);
QTRY_VERIFY(QGuiApplication::focusWindow() == &canvas);
QQuickItem *item = new TestFocusScope(canvas.rootItem());
QQuickItem *child = new TestItem(item);
QQuickItem *child2 = new TestItem;

FocusState focusState;
focusState << item << child;
FVERIFY();

item->setFocus(true);
child->setFocus(true);
focusState[child].set(true, true);
focusState[item].set(true, true);
focusState.active(child);
FVERIFY();

item->setParentItem(0);
focusState[child].set(true, false);
focusState[item].set(true, false);
focusState.active(0);
FVERIFY();

focusState.remove(child);
delete child;
item->setParentItem(canvas.rootItem());
focusState[item].set(true, true);
focusState.active(item);
FVERIFY();
}
}

void tst_qquickitem::multipleFocusClears()
Expand Down

0 comments on commit d268ac6

Please sign in to comment.