Merge remote-tracking branch 'origin/KDE/4.12'

src/kitemviews/kfileitemmodel.cpp

@@ -424,6 +424,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
@@ -969,6 +978,20 @@ void KFileItemModel::slotRefreshItems(const QList<QPair<KFileItem, KFileItem> >&
             m_items.remove(oldItem.url());
             m_items.insert(newItem.url(), index);
             indexes.append(index);
+        } else {
+            // Check if 'oldItem' is one of the filtered items.
+            QHash<KFileItem, ItemData*>::iterator it = m_filteredItems.find(oldItem);
+            if (it != m_filteredItems.end()) {
+                ItemData* itemData = it.value();
+                itemData->item = newItem;
+
+                // The data stored in 'values' might have changed. Therefore, we clear
+                // 'values' and re-populate it the next time it is requested via data(int).
+                itemData->values.clear();
+
+                m_filteredItems.erase(it);
+                m_filteredItems.insert(newItem, itemData);
+            }
         }
     }
 
@@ -1044,6 +1067,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
@@ -1207,6 +1231,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:
@@ -1216,16 +1245,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;
@@ -1234,12 +1267,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)

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();

src/tests/CMakeLists.txt

@@ -65,6 +65,7 @@ set(kfileitemmodeltest_SRCS
     testdir.cpp
     ../kitemviews/kfileitemmodel.cpp
     ../kitemviews/kitemmodelbase.cpp
+    ../kitemviews/kitemset.cpp
 )
 kde4_add_unit_test(kfileitemmodeltest TEST ${kfileitemmodeltest_SRCS})
 target_link_libraries(kfileitemmodeltest dolphinprivate ${KDE4_KIO_LIBS} ${QT_QTTEST_LIBRARY})

src/tests/kfileitemmodeltest.cpp

@@ -90,6 +90,10 @@ private slots:
     void testNameRoleGroups();
     void testNameRoleGroupsWithExpandedItems();
     void testInconsistentModel();
+    void testChangeRolesForFilteredItems();
+    void testChangeSortRoleWhileFiltering();
+    void testRefreshFilteredItems();
+    void testCreateMimeData();
 
 private:
     QStringList itemsInModel() const;
@@ -1462,6 +1466,159 @@ 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");
+}
+
+void KFileItemModelTest::testRefreshFilteredItems()
+{
+    QStringList files;
+    files << "a.txt" << "b.txt" << "c.jpg" << "d.jpg";
+    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.jpg" << "d.jpg");
+
+    const KFileItem fileItemC = m_model->fileItem(2);
+
+    // Show only the .txt files.
+    m_model->setNameFilter(".txt");
+    QCOMPARE(itemsInModel(), QStringList() << "a.txt" << "b.txt");
+
+    // Rename one of the .jpg files.
+    KFileItem fileItemE = fileItemC;
+    KUrl urlE = fileItemE.url();
+    urlE.setFileName("e.jpg");
+    fileItemE.setUrl(urlE);
+
+    m_model->slotRefreshItems(QList<QPair<KFileItem, KFileItem> >() << qMakePair(fileItemC, fileItemE));
+
+    // Show all files again, and verify that the model has updated the file name.
+    m_model->setNameFilter(QString());
+    QCOMPARE(itemsInModel(), QStringList() << "a.txt" << "b.txt" << "d.jpg" << "e.jpg");
+}
+
+void KFileItemModelTest::testCreateMimeData()
+{
+    QSet<QByteArray> modelRoles = m_model->roles();
+    modelRoles << "isExpanded" << "isExpandable" << "expandedParentsCount";
+    m_model->setRoles(modelRoles);
+
+    QStringList files;
+    files << "a/1";
+    m_testDir->createFiles(files);
+
+    m_model->loadDirectory(m_testDir->url());
+    QVERIFY(QTest::kWaitForSignal(m_model, SIGNAL(itemsInserted(KItemRangeList)), DefaultTimeout));
+    QCOMPARE(itemsInModel(), QStringList() << "a");
+
+    // Expand "a/".
+    m_model->setExpanded(0, true);
+    QVERIFY(QTest::kWaitForSignal(m_model, SIGNAL(itemsInserted(KItemRangeList)), DefaultTimeout));
+    QCOMPARE(itemsInModel(), QStringList() << "a" << "1");
+
+    // Verify that creating the MIME data for a child of an expanded folder does
+    // not cause a crash, see https://bugs.kde.org/show_bug.cgi?id=329119
+    KItemSet selection;
+    selection.insert(1);
+    QMimeData* mimeData = m_model->createMimeData(selection);
+    delete mimeData;
+}
+
 QStringList KFileItemModelTest::itemsInModel() const
 {
     QStringList items;