From: Andrew den Exter Date: Fri, 25 Nov 2011 03:33:09 +0000 (+1000) Subject: Fix searchs for insert positions in QDeclarativeListCompositor. X-Git-Tag: qt-v5.0.0-alpha1~1025 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=900137ea6a593f612e091cf629fc3a0ec929fb5f;p=profile%2Fivi%2Fqtdeclarative.git Fix searchs for insert positions in QDeclarativeListCompositor. When scanning for a start range don't stop on ranges that don't have an group flags unless that range is the terminal range. This fixes a couple of issues where moving an item to the end of the list would position it after a prepend only range instead of before it, or would miscalculate the iterator offset resulting in invalid indexes in the insert range. Change-Id: Ic4aa001edf43ec86a65d432cd8f80abf0b44d276 Reviewed-by: Martin Jones --- diff --git a/src/declarative/util/qdeclarativelistcompositor.cpp b/src/declarative/util/qdeclarativelistcompositor.cpp index 02db70f..d73d76e 100644 --- a/src/declarative/util/qdeclarativelistcompositor.cpp +++ b/src/declarative/util/qdeclarativelistcompositor.cpp @@ -199,7 +199,7 @@ QDeclarativeListCompositor::iterator &QDeclarativeListCompositor::iterator::oper QDeclarativeListCompositor::insert_iterator &QDeclarativeListCompositor::insert_iterator::operator +=(int difference) { Q_ASSERT(difference >= 0); - while (!(range->flags & groupFlag) && (range->flags & (GroupMask | CacheFlag))) { + while (!(range->flags & groupFlag)) { incrementIndexes(range->count - offset); offset = 0; range = range->next; @@ -221,10 +221,10 @@ QDeclarativeListCompositor::insert_iterator &QDeclarativeListCompositor::insert_ QDeclarativeListCompositor::insert_iterator &QDeclarativeListCompositor::insert_iterator::operator -=(int difference) { Q_ASSERT(difference >= 0); - while (!(range->flags & groupFlag) && (range->flags & (GroupMask | CacheFlag))) { + while (!(range->flags & groupFlag) && range->previous->flags) { decrementIndexes(offset); range = range->previous; - offset = range->count; + offset = (range->flags & (GroupMask | CacheFlag)) ? range->count : 0; } decrementIndexes(offset); offset -= difference; diff --git a/tests/auto/declarative/qdeclarativelistcompositor/tst_qdeclarativelistcompositor.cpp b/tests/auto/declarative/qdeclarativelistcompositor/tst_qdeclarativelistcompositor.cpp index b3b3aaa..ed786eb 100644 --- a/tests/auto/declarative/qdeclarativelistcompositor/tst_qdeclarativelistcompositor.cpp +++ b/tests/auto/declarative/qdeclarativelistcompositor/tst_qdeclarativelistcompositor.cpp @@ -159,6 +159,7 @@ private slots: void setFlags(); void move_data(); void move(); + void moveFromEnd(); void clear(); void listItemsInserted_data(); void listItemsInserted(); @@ -937,6 +938,25 @@ void tst_qdeclarativelistcompositor::move_data() << IndexArray(defaultIndexes) << ListArray(defaultLists) << IndexArray(visibleIndexes) << ListArray(visibleLists) << IndexArray() << ListArray(); + } { static const int cacheIndexes[] = {0,1}; + static const void *cacheLists[] = {a,a}; + static const int defaultIndexes[] = {0,1}; + static const void *defaultLists[] = {a,a}; + QTest::newRow("0, 1, 1") + << (RangeList() + << Range(a, 0, 1, C::PrependFlag) + << Range(a, 1, 1, C::PrependFlag | C::DefaultFlag | C::CacheFlag) + << Range(a, 2, 0, C::AppendFlag | C::PrependFlag) + << Range(a, 0, 1, C::DefaultFlag | C::CacheFlag)) + << C::Default << 0 << C::Default << 1 << 1 + << (RemoveList() + << Remove(0, 0, 0, 0, 1, C::DefaultFlag | C::CacheFlag, 0)) + << (InsertList() + << Insert(0, 0, 1, 1, 1, C::DefaultFlag | C::CacheFlag, 0)) + << IndexArray(cacheIndexes) << ListArray(cacheLists) + << IndexArray(defaultIndexes) << ListArray(defaultLists) + << IndexArray() << ListArray() + << IndexArray() << ListArray(); } } @@ -1002,6 +1022,33 @@ void tst_qdeclarativelistcompositor::move() QCOMPARE(it.modelIndex(), selectionIndexes[i]); } } + +void tst_qdeclarativelistcompositor::moveFromEnd() +{ + int listA; void *a = &listA; + + QDeclarativeListCompositor compositor; + compositor.append(a, 0, 1, C::AppendFlag | C::PrependFlag | C::DefaultFlag); + + // Moving an item anchors it to that position. + compositor.move(C::Default, 0, C::Default, 0, 1); + + // The existing item is anchored at 0 so prepending an item to the source will append it here + QVector inserts; + compositor.listItemsInserted(a, 0, 1, &inserts); + + QCOMPARE(inserts.count(), 1); + QCOMPARE(inserts.at(0).index[1], 1); + QCOMPARE(inserts.at(0).count, 1); + + C::iterator it; + it = compositor.find(C::Default, 0); + QCOMPARE(it.modelIndex(), 1); + + it = compositor.find(C::Default, 1); + QCOMPARE(it.modelIndex(), 0); +} + void tst_qdeclarativelistcompositor::clear() { QDeclarativeListCompositor compositor;