3 files changed, 130 insertions, 7 deletions

diff --git a/src/kitemviews/kfileitemmodel.cpp b/src/kitemviews/kfileitemmodel.cpp
index 4c8577543..739384bf9 100644
--- a/src/kitemviews/kfileitemmodel.cpp
+++ b/src/kitemviews/kfileitemmodel.cpp
@@ -413,6 +413,15 @@ void KFileItemModel::setRoles(const QSet<QByteArray>& roles)
         kWarning() << "TODO: Emitting itemsChanged() with no information what has changed!";
         emit itemsChanged(KItemRangeList() << KItemRange(0, count()), QSet<QByteArray>());
     }
+
+    // Clear the 'values' of all filtered items. They will be re-populated with the
+    // correct roles the next time 'values' will be accessed via data(int).
+    QHash<KFileItem, ItemData*>::iterator filteredIt = m_filteredItems.begin();
+    const QHash<KFileItem, ItemData*>::iterator filteredEnd = m_filteredItems.end();
+    while (filteredIt != filteredEnd) {
+        (*filteredIt)->values.clear();
+        ++filteredIt;
+    }
 }
 
 QSet<QByteArray> KFileItemModel::roles() const
@@ -1033,6 +1042,7 @@ void KFileItemModel::insertItems(QList<ItemData*>& newItems)
 #endif
 
     m_groups.clear();
+    prepareItemsForSorting(newItems);
 
     if (m_sortRole == NameRole && m_naturalSorting) {
         // Natural sorting of items can be very slow. However, it becomes much
@@ -1196,6 +1206,11 @@ QList<KFileItemModel::ItemData*> KFileItemModel::createItemDataList(const KUrl&
         itemDataList.append(itemData);
     }
 
+    return itemDataList;
+}
+
+void KFileItemModel::prepareItemsForSorting(QList<ItemData*>& itemDataList)
+{
     switch (m_sortRole) {
     case PermissionsRole:
     case OwnerRole:
@@ -1205,16 +1220,20 @@ QList<KFileItemModel::ItemData*> KFileItemModel::createItemDataList(const KUrl&
         // These roles can be determined with retrieveData, and they have to be stored
         // in the QHash "values" for the sorting.
         foreach (ItemData* itemData, itemDataList) {
-            itemData->values = retrieveData(itemData->item, parentItem);
+            if (itemData->values.isEmpty()) {
+                itemData->values = retrieveData(itemData->item, itemData->parent);
+            }
         }
         break;
 
     case TypeRole:
         // At least store the data including the file type for items with known MIME type.
         foreach (ItemData* itemData, itemDataList) {
-            const KFileItem item = itemData->item;
-            if (item.isDir() || item.isMimeTypeKnown()) {
-                itemData->values = retrieveData(itemData->item, parentItem);
+            if (itemData->values.isEmpty()) {
+                const KFileItem item = itemData->item;
+                if (item.isDir() || item.isMimeTypeKnown()) {
+                    itemData->values = retrieveData(itemData->item, itemData->parent);
+                }
             }
         }
         break;
@@ -1223,12 +1242,10 @@ QList<KFileItemModel::ItemData*> KFileItemModel::createItemDataList(const KUrl&
         // The other roles are either resolved by KFileItemModelRolesUpdater
         // (this includes the SizeRole for directories), or they do not need
         // to be stored in the QHash "values" for sorting because the data can
-        // be retrieved directly from the KFileItem (NameRole, SiezRole for files,
+        // be retrieved directly from the KFileItem (NameRole, SizeRole for files,
         // DateRole).
         break;
     }
-
-    return itemDataList;
 }
 
 int KFileItemModel::expandedParentsCount(const ItemData* data)
diff --git a/src/kitemviews/kfileitemmodel.h b/src/kitemviews/kfileitemmodel.h
index c57329fc8..022429023 100644
--- a/src/kitemviews/kfileitemmodel.h
+++ b/src/kitemviews/kfileitemmodel.h
@@ -320,6 +320,13 @@ private:
      */
     QList<ItemData*> createItemDataList(const KUrl& parentUrl, const KFileItemList& items) const;
 
+    /**
+     * Prepares the items for sorting. Normally, the hash 'values' in ItemData is filled
+     * lazily to save time and memory, but for some sort roles, it is expected that the
+     * sort role data is stored in 'values'.
+     */
+    void prepareItemsForSorting(QList<ItemData*>& itemDataList);
+
     static int expandedParentsCount(const ItemData* data);
 
     void removeExpandedItems();
diff --git a/src/tests/kfileitemmodeltest.cpp b/src/tests/kfileitemmodeltest.cpp
index 62ff4fae2..ede9a3d6e 100644
--- a/src/tests/kfileitemmodeltest.cpp
+++ b/src/tests/kfileitemmodeltest.cpp
@@ -90,6 +90,8 @@ private slots:
     void testNameRoleGroups();
     void testNameRoleGroupsWithExpandedItems();
     void testInconsistentModel();
+    void testChangeRolesForFilteredItems();
+    void testChangeSortRoleWhileFiltering();
 
 private:
     QStringList itemsInModel() const;
@@ -1462,6 +1464,103 @@ void KFileItemModelTest::testInconsistentModel()
 
 }
 
+void KFileItemModelTest::testChangeRolesForFilteredItems()
+{
+    QSet<QByteArray> modelRoles = m_model->roles();
+    modelRoles << "owner";
+    m_model->setRoles(modelRoles);
+
+    QStringList files;
+    files << "a.txt" << "aa.txt" << "aaa.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" << "aa.txt" << "aaa.txt");
+
+    for (int index = 0; index < m_model->count(); ++index) {
+        // All items should have the "text" and "owner" roles, but not "group".
+        QVERIFY(m_model->data(index).contains("text"));
+        QVERIFY(m_model->data(index).contains("owner"));
+        QVERIFY(!m_model->data(index).contains("group"));
+    }
+
+    // Add a filter, such that only "aaa.txt" remains in the model.
+    m_model->setNameFilter("aaa");
+    QCOMPARE(itemsInModel(), QStringList() << "aaa.txt");
+
+    // Add the "group" role.
+    modelRoles << "group";
+    m_model->setRoles(modelRoles);
+
+    // Modify the filter, such that "aa.txt" reappears, and verify that all items have the expected roles.
+    m_model->setNameFilter("aa");
+    QCOMPARE(itemsInModel(), QStringList() << "aa.txt" << "aaa.txt");
+
+    for (int index = 0; index < m_model->count(); ++index) {
+        // All items should have the "text", "owner", and "group" roles.
+        QVERIFY(m_model->data(index).contains("text"));
+        QVERIFY(m_model->data(index).contains("owner"));
+        QVERIFY(m_model->data(index).contains("group"));
+    }
+
+    // Remove the "owner" role.
+    modelRoles.remove("owner");
+    m_model->setRoles(modelRoles);
+
+    // Clear the filter, and verify that all items have the expected roles
+    m_model->setNameFilter(QString());
+    QCOMPARE(itemsInModel(), QStringList() << "a.txt" << "aa.txt" << "aaa.txt");
+
+    for (int index = 0; index < m_model->count(); ++index) {
+        // All items should have the "text" and "group" roles, but now "owner".
+        QVERIFY(m_model->data(index).contains("text"));
+        QVERIFY(!m_model->data(index).contains("owner"));
+        QVERIFY(m_model->data(index).contains("group"));
+    }
+}
+
+void KFileItemModelTest::testChangeSortRoleWhileFiltering()
+{
+    KFileItemList items;
+
+    KIO::UDSEntry entry;
+    entry.insert(KIO::UDSEntry::UDS_FILE_TYPE, 0100000);    // S_IFREG might not be defined on non-Unix platforms.
+    entry.insert(KIO::UDSEntry::UDS_ACCESS, 07777);
+    entry.insert(KIO::UDSEntry::UDS_SIZE, 0);
+    entry.insert(KIO::UDSEntry::UDS_MODIFICATION_TIME, 0);
+    entry.insert(KIO::UDSEntry::UDS_GROUP, "group");
+    entry.insert(KIO::UDSEntry::UDS_ACCESS_TIME, 0);
+
+    entry.insert(KIO::UDSEntry::UDS_NAME, "a.txt");
+    entry.insert(KIO::UDSEntry::UDS_USER, "user-b");
+    items.append(KFileItem(entry, m_testDir->url(), false, true));
+
+    entry.insert(KIO::UDSEntry::UDS_NAME, "b.txt");
+    entry.insert(KIO::UDSEntry::UDS_USER, "user-c");
+    items.append(KFileItem(entry, m_testDir->url(), false, true));
+
+    entry.insert(KIO::UDSEntry::UDS_NAME, "c.txt");
+    entry.insert(KIO::UDSEntry::UDS_USER, "user-a");
+    items.append(KFileItem(entry, m_testDir->url(), false, true));
+
+    m_model->slotItemsAdded(m_testDir->url(), items);
+    m_model->slotCompleted();
+
+    QCOMPARE(itemsInModel(), QStringList() << "a.txt" << "b.txt" << "c.txt");
+
+    // Add a filter.
+    m_model->setNameFilter("a");
+    QCOMPARE(itemsInModel(), QStringList() << "a.txt");
+
+    // Sort by "owner".
+    m_model->setSortRole("owner");
+
+    // Clear the filter, and verify that the items are sorted correctly.
+    m_model->setNameFilter(QString());
+    QCOMPARE(itemsInModel(), QStringList() << "c.txt" << "a.txt" << "b.txt");
+}
+
 QStringList KFileItemModelTest::itemsInModel() const
 {
     QStringList items;