Make sure that the sort order is correct after renaming

KFileItemModel::setData() should not only cause a resorting when the
sort role is changed. The name is always used as a fallback if the sort
role of multiple files is equal, therefore, renaming a file can change
the correct order of the files even if the files are not sorted by
"name".

Unit test included.

BUG: 323518
FIXED-IN: 4.11.1
REVIEW: 111721

diff --git a/src/kitemviews/kfileitemmodel.cpp b/src/kitemviews/kfileitemmodel.cpp
index c0dedff396d98f375358e1439a7cba994ce906d0..70e8834a665fabfea16a4ca9533a9322ad877884 100644 (file)
--- a/src/kitemviews/kfileitemmodel.cpp
+++ b/src/kitemviews/kfileitemmodel.cpp
@@ -192,8 +192,30 @@ bool KFileItemModel::setData(int index, const QHash<QByteArray, QVariant>& value
 
     emit itemsChanged(KItemRangeList() << KItemRange(index, 1), changedRoles);
 
-    if (changedRoles.contains(sortRole())) {
-        m_resortAllItemsTimer->start();
+    // Trigger a resorting if the item's correct position has changed. Note
+    // that this can happen even if the sort role has not changed at all
+    // because the file name can be used as a fallback.
+    if (changedRoles.contains(sortRole()) || changedRoles.contains(roleForType(NameRole))) {
+        // Compare the changed item with its neighbors to see
+        // if an expensive resorting is needed at all.
+        const ItemData* changedItem = m_itemData.at(index);
+        const ItemData* previousItem = (index == 0) ? 0 : m_itemData.at(index - 1);
+        const ItemData* nextItem = (index == m_itemData.count() - 1) ? 0 : m_itemData.at(index + 1);
+
+        if ((previousItem && lessThan(changedItem, previousItem))
+            || (nextItem && lessThan(nextItem, changedItem))) {
+            m_resortAllItemsTimer->start();
+        } else if (groupedSorting() && changedRoles.contains(sortRole())) {
+            // The position is still correct, but the groups might have changed
+            // if the changed item is either the first or the last item in a
+            // group.
+            // In principle, we could try to find out if the item really is the
+            // first or last one in its group and then update the groups
+            // (possibly with a delayed timer to make sure that we don't
+            // re-calculate the groups very often if items are updated one by
+            // one), but starting m_resortAllItemsTimer is easier.
+            m_resortAllItemsTimer->start();
+        }
     }
 
     return true;

diff --git a/src/tests/kfileitemmodeltest.cpp b/src/tests/kfileitemmodeltest.cpp
index 513ecef5a0f65cd19067ad9d0075f1ffbf619db5..f8439789bd1875cf9c2d65af30ea833391539778 100644 (file)
--- a/src/tests/kfileitemmodeltest.cpp
+++ b/src/tests/kfileitemmodeltest.cpp
@@ -70,6 +70,7 @@ private slots:
     void testSetDataWithModifiedSortRole_data();
     void testSetDataWithModifiedSortRole();
     void testChangeSortRole();
+    void testResortAfterChangingName();
     void testModelConsistencyWhenInsertingItems();
     void testItemRangeConsistencyWhenInsertingItems();
     void testExpandItems();
@@ -368,6 +369,30 @@ void KFileItemModelTest::testChangeSortRole()
     QVERIFY(ok1 || ok2);
 }
 
+void KFileItemModelTest::testResortAfterChangingName()
+{
+    // We sort by size in a directory where all files have the same size.
+    // Therefore, the files are sorted by their names.
+    m_model->setSortRole("size");
+
+    QStringList files;
+    files << "a.txt" << "b.txt" << "c.txt";
+    m_testDir->createFiles(files);
+
+    m_model->loadDirectory(m_testDir->url());
+    QVERIFY(QTest::kWaitForSignal(m_model, SIGNAL(itemsInserted(KItemRangeList)), DefaultTimeout));
+    QCOMPARE(itemsInModel(), QStringList() << "a.txt" << "b.txt" << "c.txt");
+
+    // We rename a.txt to d.txt. Even though the size has not changed at all,
+    // the model must re-sort the items.
+    QHash<QByteArray, QVariant> data;
+    data.insert("text", "d.txt");
+    m_model->setData(0, data);
+
+    QVERIFY(QTest::kWaitForSignal(m_model, SIGNAL(itemsMoved(KItemRange,QList<int>)), DefaultTimeout));
+    QCOMPARE(itemsInModel(), QStringList() << "b.txt" << "c.txt" << "d.txt");
+}
+
 void KFileItemModelTest::testModelConsistencyWhenInsertingItems()
 {
     //QSKIP("Temporary disabled", SkipSingle);