DelayRemove of list delegate on section boundary duplicated section

When removing a delegate with a removal animation that fell on a
section boundary (i.e. owned the section header), the following
item would also create a section header before the previous item was
removed.  Make updateSections() include the removed, but visible items
in its update.  Ensure updateSections() is called when the removed
item is destroyed to ensure a new section header is created at that
point.

Change-Id: Ie831e3acf65b2989ebb030e2ab38cdbe179a9d45
Task-number: QTBUG-17606
Reviewed-by: Michael Brasser

3 files changed, 47 insertions, 8 deletions

diff --git a/src/declarative/graphicsitems/qdeclarativelistview.cpp b/src/declarative/graphicsitems/qdeclarativelistview.cpp
index 91de5a6..338cb58 100644
--- a/src/declarative/graphicsitems/qdeclarativelistview.cpp
+++ b/src/declarative/graphicsitems/qdeclarativelistview.cpp
@@ -228,6 +228,26 @@ public:
         return 0;
     }
 
+    // Returns the item before modelIndex, if created.
+    // May return an item marked for removal.
+    FxListItem *itemBefore(int modelIndex) const {
+        if (modelIndex < visibleIndex)
+            return 0;
+        int idx = 1;
+        int lastIndex = -1;
+        while (idx < visibleItems.count()) {
+            FxListItem *item = visibleItems.at(idx);
+            if (item->index != -1)
+                lastIndex = item->index;
+            if (item->index == modelIndex)
+                return visibleItems.at(idx-1);
+            ++idx;
+        }
+        if (lastIndex == modelIndex-1)
+            return visibleItems.last();
+        return 0;
+    }
+
     qreal position() const {
         Q_Q(const QDeclarativeListView);
         return orient == QDeclarativeListView::Vertical ? q->contentY() : q->contentX();
@@ -561,7 +581,7 @@ FxListItem *QDeclarativeListViewPrivate::createItem(int modelIndex)
             QString propValue = model->stringValue(modelIndex, sectionCriteria->property());
             listItem->attached->m_section = sectionCriteria->sectionString(propValue);
             if (modelIndex > 0) {
-                if (FxListItem *item = visibleItem(modelIndex-1))
+                if (FxListItem *item = itemBefore(modelIndex))
                     listItem->attached->m_prevSection = item->attached->section();
                 else
                     listItem->attached->m_prevSection = sectionAt(modelIndex-1);
@@ -969,18 +989,18 @@ void QDeclarativeListViewPrivate::updateSections()
         QDeclarativeListViewAttached *prevAtt = 0;
         int idx = -1;
         for (int i = 0; i < visibleItems.count(); ++i) {
+            QDeclarativeListViewAttached *attached = visibleItems.at(i)->attached;
+            attached->setPrevSection(prevSection);
             if (visibleItems.at(i)->index != -1) {
-                QDeclarativeListViewAttached *attached = visibleItems.at(i)->attached;
-                attached->setPrevSection(prevSection);
                 QString propValue = model->stringValue(visibleItems.at(i)->index, sectionCriteria->property());
                 attached->setSection(sectionCriteria->sectionString(propValue));
-                if (prevAtt)
-                    prevAtt->setNextSection(attached->section());
-                createSection(visibleItems.at(i));
-                prevSection = attached->section();
-                prevAtt = attached;
                 idx = visibleItems.at(i)->index;
             }
+            createSection(visibleItems.at(i));
+            if (prevAtt)
+                prevAtt->setNextSection(attached->section());
+            prevSection = attached->section();
+            prevAtt = attached;
         }
         if (prevAtt) {
             if (idx > 0 && idx < model->count()-1)
@@ -3096,6 +3116,7 @@ void QDeclarativeListView::destroyRemoved()
     }
 
     // Correct the positioning of the items
+    d->updateSections();
     d->layout();
 }
 
diff --git a/tests/auto/declarative/qdeclarativelistview/data/listview-sections_delegate.qml b/tests/auto/declarative/qdeclarativelistview/data/listview-sections_delegate.qml
index 35a398b..9d9cda8 100644
--- a/tests/auto/declarative/qdeclarativelistview/data/listview-sections_delegate.qml
+++ b/tests/auto/declarative/qdeclarativelistview/data/listview-sections_delegate.qml
@@ -41,6 +41,11 @@ Rectangle {
                         text: wrapper.y
                     }
                 }
+                ListView.onRemove: SequentialAnimation {
+                    PropertyAction { target: wrapper; property: "ListView.delayRemove"; value: true }
+                    NumberAnimation { target: wrapper; property: "height"; to: 0; duration: 100; easing.type: Easing.InOutQuad }
+                    PropertyAction { target: wrapper; property: "ListView.delayRemove"; value: false }
+                }
             }
         }
     ]
diff --git a/tests/auto/declarative/qdeclarativelistview/tst_qdeclarativelistview.cpp b/tests/auto/declarative/qdeclarativelistview/tst_qdeclarativelistview.cpp
index f358625..c7f90da 100644
--- a/tests/auto/declarative/qdeclarativelistview/tst_qdeclarativelistview.cpp
+++ b/tests/auto/declarative/qdeclarativelistview/tst_qdeclarativelistview.cpp
@@ -1078,6 +1078,19 @@ void tst_QDeclarativeListView::sectionsDelegate()
         QTRY_COMPARE(item->y(), qreal(i*20*6));
     }
 
+    // remove section boundary
+    model.removeItem(5);
+    qApp->processEvents();
+    for (int i = 0; i < 3; ++i) {
+        QDeclarativeItem *item = findItem<QDeclarativeItem>(contentItem,
+                "sect_" + (i == 0 ? QString("aaa") : QString::number(i)));
+        QVERIFY(item);
+    }
+
+    // QTBUG-17606
+    QList<QDeclarativeItem*> items = findItems<QDeclarativeItem>(contentItem, "sect_1");
+    QCOMPARE(items.count(), 1);
+
     delete canvas;
 }