From 13eb07b3a63d53abcabfa60659fd829051db66a3 Mon Sep 17 00:00:00 2001 From: Andrew den Exter Date: Tue, 13 Dec 2011 17:17:56 +1000 Subject: [PATCH] Fix crash when items are moved in model. When erasing an item we need to backtrack to the previous item so the next iteration doesn't skip an item. In the worst case the next item is the last and a failure to backtrack will cause the loop to wrap around and run over the list again. Task-number: QTBUG-23107 Change-Id: I82156f6fc1f7973ba11f09a4694230c77c293757 Reviewed-by: Bea Lam --- src/quick/util/qdeclarativelistcompositor.cpp | 35 +++++++++----------- .../tst_qdeclarativelistcompositor.cpp | 17 +++++++++ 2 files changed, 33 insertions(+), 19 deletions(-) diff --git a/src/quick/util/qdeclarativelistcompositor.cpp b/src/quick/util/qdeclarativelistcompositor.cpp index d73d76e..e43a8a7 100644 --- a/src/quick/util/qdeclarativelistcompositor.cpp +++ b/src/quick/util/qdeclarativelistcompositor.cpp @@ -872,9 +872,7 @@ void QDeclarativeListCompositor::listItemsRemoved( { QT_DECLARATIVE_TRACE_LISTCOMPOSITOR(<< list << *removals) - for (iterator it(m_ranges.next, 0, Default, m_groupCount); - *it != &m_ranges && !removals->isEmpty(); - *it = it->next) { + for (iterator it(m_ranges.next, 0, Default, m_groupCount); *it != &m_ranges; *it = it->next) { if (it->list != list || it->flags == CacheFlag) { it.incrementIndexes(it->count); continue; @@ -920,21 +918,20 @@ void QDeclarativeListCompositor::listItemsRemoved( translatedRemoval.moveId = ++moveId; movedFlags->append(MovedFlags(moveId, it->flags & ~AppendFlag)); - removal = removals->insert(removal, QDeclarativeChangeSet::Remove( - removal->index, removeCount, translatedRemoval.moveId)); - ++removal; - insertion = insertions->insert(insertion, QDeclarativeChangeSet::Insert( - insertion->index, removeCount, translatedRemoval.moveId)); - ++insertion; - - removal->count -= removeCount; - insertion->index += removeCount; - insertion->count -= removeCount; - if (removal->count == 0) { - removal = removals->erase(removal); - insertion = insertions->erase(insertion); - --removal; - --insertion; + if (removeCount < removal->count) { + removal = removals->insert(removal, QDeclarativeChangeSet::Remove( + removal->index, removeCount, translatedRemoval.moveId)); + ++removal; + insertion = insertions->insert(insertion, QDeclarativeChangeSet::Insert( + insertion->index, removeCount, translatedRemoval.moveId)); + ++insertion; + + removal->count -= removeCount; + insertion->index += removeCount; + insertion->count -= removeCount; + } else { + removal->moveId = translatedRemoval.moveId; + insertion->moveId = translatedRemoval.moveId; } } else { if (offset > 0) { @@ -989,7 +986,7 @@ void QDeclarativeListCompositor::listItemsRemoved( it->previous->count += it->count; it->previous->flags = it->flags; it.incrementIndexes(it->count); - *it = erase(*it); + *it = erase(*it)->previous; removed = true; } } diff --git a/tests/auto/declarative/qdeclarativelistcompositor/tst_qdeclarativelistcompositor.cpp b/tests/auto/declarative/qdeclarativelistcompositor/tst_qdeclarativelistcompositor.cpp index ed786eb..43ae2f6 100644 --- a/tests/auto/declarative/qdeclarativelistcompositor/tst_qdeclarativelistcompositor.cpp +++ b/tests/auto/declarative/qdeclarativelistcompositor/tst_qdeclarativelistcompositor.cpp @@ -1490,6 +1490,23 @@ void tst_qdeclarativelistcompositor::listItemsMoved_data() << IndexArray(defaultIndexes) << IndexArray() << IndexArray(); + } { static const int cacheIndexes[] = {/*A*/0,1,2,3}; + static const int defaultIndexes[] = {/*A*/0,1,2,3}; + static const int selectionIndexes[] = {/*A*/3}; + QTest::newRow("Move selection") + << (RangeList() + << Range(a, 0, 2, C::PrependFlag | C::DefaultFlag | C::CacheFlag) + << Range(a, 2, 1, C::PrependFlag | SelectionFlag | C::DefaultFlag | C::CacheFlag) + << Range(a, 3, 1, C::AppendFlag | C::PrependFlag | C::DefaultFlag | C::CacheFlag)) + << a << 2 << 3 << 1 + << (RemoveList() + << Remove(0, 0, 2, 2, 1, C::PrependFlag | SelectionFlag | C::DefaultFlag | C::CacheFlag, 0)) + << (InsertList() + << Insert(0, 0, 3, 3, 1, C::PrependFlag | SelectionFlag | C::DefaultFlag | C::CacheFlag, 0)) + << IndexArray(cacheIndexes) + << IndexArray(defaultIndexes) + << IndexArray() + << IndexArray(selectionIndexes); } } -- 1.7.2.5