Rewrite filter algorithm to properly support filtering with expanded folders under Detail View mode.

BUG: 411878
CCBUG: 442275
FIXED-IN: 21.12

src/kitemviews/kfileitemmodel.cpp

@@ -695,45 +695,87 @@ QStringList KFileItemModel::mimeTypeFilters() const
     return m_filter.mimeTypes();
 }
 
-
 void KFileItemModel::applyFilters()
 {
-    // Check which shown items from m_itemData must get
+    // ===STEP 1===
-    // hidden and hence moved to m_filteredItems.
+    // Check which previously shown items from m_itemData must now get
-    QVector<int> newFilteredIndexes;
+    // hidden and hence moved from m_itemData into m_filteredItems.
 
-    const int itemCount = m_itemData.count();
+    QList<int> newFilteredIndexes; // This structure is good for prepending. We will want an ascending sorted Container at the end, this will do fine.
-    for (int index = 0; index < itemCount; ++index) {
-        ItemData* itemData = m_itemData.at(index);
 
-        // Only filter non-expanded items as child items may never
+    // This pointer will refer to the next confirmed shown item from the point of
-        // exist without a parent item
+    // view of the current "itemData" in the upcoming "for" loop.
-        if (!itemData->values.value("isExpanded").toBool()) {
+    ItemData *itemShownBelow = nullptr;
-            const KFileItem item = itemData->item;
+
-            if (!m_filter.matches(item)) {
+    // We will iterate backwards because it's convenient to know beforehand if the item just below is its child or not.
-                newFilteredIndexes.append(index);
+    for (int index = m_itemData.count() - 1; index >= 0; --index) {
-                m_filteredItems.insert(item, itemData);
+        ItemData *itemData = m_itemData.at(index);
-            }
+
+        if (m_filter.matches(itemData->item)
+            || (itemShownBelow && itemShownBelow->parent == itemData && itemData->values.value("isExpanded").toBool())) {
+            // We could've entered here for two reasons:
+            // 1. This item passes the filter itself
+            // 2. This is an expanded folder that doesn't pass the filter but sees a filter-passing child just below
+
+            // So this item must remain shown.
+            // Lets register this item as the next shown item from the point of view of the next iteration of this for loop
+            itemShownBelow = itemData;
+        } else {
+            // We hide this item for now, however, for expanded folders this is not final:
+            // if after the next "for" loop we discover that its children must now be shown with the newly applied fliter, we shall re-insert it
+            newFilteredIndexes.prepend(index);
+            m_filteredItems.insert(itemData->item, itemData);
+            // indexShownBelow doesn't get updated since this item will be hidden
         }
     }
 
-    const KItemRangeList removedRanges = KItemRangeList::fromSortedContainer(newFilteredIndexes);
+    // This will remove the newly filtered items from m_itemData
-    removeItems(removedRanges, KeepItemData);
+    removeItems(KItemRangeList::fromSortedContainer(newFilteredIndexes), KeepItemData);
 
+    // ===STEP 2===
     // Check which hidden items from m_filteredItems should
-    // get visible again and hence removed from m_filteredItems.
+    // become visible again and hence moved from m_filteredItems back into m_itemData.
-    QList<ItemData*> newVisibleItems;
 
-    QHash<KFileItem, ItemData*>::iterator it = m_filteredItems.begin();
+    QList<ItemData *> newVisibleItems;
+
+    QHash<KFileItem, ItemData *> ancestorsOfNewVisibleItems; // We will make sure these also become visible in step 3.
+
+    QHash<KFileItem, ItemData *>::iterator it = m_filteredItems.begin();
     while (it != m_filteredItems.end()) {
         if (m_filter.matches(it.key())) {
             newVisibleItems.append(it.value());
+
+            // If this is a child of an expanded folder, we must make sure that its whole parental chain will also be shown.
+            // We will go up through its parental chain until we either:
+            // 1 - reach the "root item" of the current view, i.e the currently opened folder on Dolphin. Their children have their ItemData::parent set to nullptr.
+            // or
+            // 2 - we reach an unfiltered parent or a previously discovered ancestor.
+            for (ItemData *parent = it.value()->parent; parent && !ancestorsOfNewVisibleItems.contains(parent->item) && m_filteredItems.contains(parent->item);
+                 parent = parent->parent) {
+                // We wish we could remove this parent from m_filteredItems right now, but we are iterating over it
+                // and it would mess up the iteration. We will mark it to be removed in step 3.
+                ancestorsOfNewVisibleItems.insert(parent->item, parent);
+            }
+
             it = m_filteredItems.erase(it);
         } else {
+            // Item remains filtered for now
+            // However, for expanded folders this is not final, we may discover later that it has unfiltered descendants.
             ++it;
         }
     }
 
+    // ===STEP 3===
+    // Handles the ancestorsOfNewVisibleItems.
+    // Now that we are done iterating through m_filteredItems we can safely move the ancestorsOfNewVisibleItems from m_filteredItems to newVisibleItems.
+    for (it = ancestorsOfNewVisibleItems.begin(); it != ancestorsOfNewVisibleItems.end(); it++) {
+        if (m_filteredItems.remove(it.key())) {
+            // m_filteredItems still contained this ancestor until now so we can be sure that we aren't adding a duplicate ancestor to newVisibleItems.
+            newVisibleItems.append(it.value());
+        }
+    }
+
+    // This will insert the newly discovered unfiltered items into m_itemData
     insertItems(newVisibleItems);
 }
 

src/tests/kfileitemmodeltest.cpp

@@ -1226,11 +1226,16 @@ void KFileItemModelTest::collapseParentOfHiddenItems()
     QVERIFY(itemsInsertedSpy.wait());
     QCOMPARE(m_model->count(), 7); // 7 items: "a/", "a/b/", "a/b/c", "a/b/c/d/", "a/b/c/1", "a/b/1", "a/1"
 
-    // Set a name filter that matches nothing -> only the expanded folders remain.
+    // Set a name filter that matches nothing -> nothing should remain.
     m_model->setNameFilter("xyz");
     QCOMPARE(itemsRemovedSpy.count(), 1);
-    QCOMPARE(m_model->count(), 3);
+    QCOMPARE(m_model->count(), 0); //Everything is hidden
-    QCOMPARE(itemsInModel(), QStringList() << "a" << "b" << "c");
+    QCOMPARE(itemsInModel(), QStringList());
+
+    //Filter by the file names. Folder "d" will be hidden since it was collapsed
+    m_model->setNameFilter("1");
+    QCOMPARE(itemsRemovedSpy.count(), 1); // nothing was removed, itemsRemovedSpy count will remain the same:
+    QCOMPARE(m_model->count(), 6); // 6 items: "a/", "a/b/", "a/b/c", "a/b/c/1", "a/b/1", "a/1"
 
     // Collapse the folder "a/".
     m_model->setExpanded(0, false);
@@ -1238,9 +1243,11 @@ void KFileItemModelTest::collapseParentOfHiddenItems()
     QCOMPARE(m_model->count(), 1);
     QCOMPARE(itemsInModel(), QStringList() << "a");
 
-    // Remove the filter -> no files should appear (and we should not get a crash).
+    // Remove the filter -> "a" should still appear (and we should not get a crash).
     m_model->setNameFilter(QString());
+    QCOMPARE(itemsRemovedSpy.count(), 2); // nothing was removed, itemsRemovedSpy count will remain the same:
     QCOMPARE(m_model->count(), 1);
+    QCOMPARE(itemsInModel(), QStringList() << "a");
 }
 
 /**
@@ -1276,9 +1283,15 @@ void KFileItemModelTest::removeParentOfHiddenItems()
     QVERIFY(itemsInsertedSpy.wait());
     QCOMPARE(m_model->count(), 7); // 7 items: "a/", "a/b/", "a/b/c", "a/b/c/d/", "a/b/c/1", "a/b/1", "a/1"
 
-    // Set a name filter that matches nothing -> only the expanded folders remain.
+    // Set a name filter that matches nothing -> nothing should remain.
     m_model->setNameFilter("xyz");
     QCOMPARE(itemsRemovedSpy.count(), 1);
+    QCOMPARE(m_model->count(), 0);
+    QCOMPARE(itemsInModel(), QStringList());
+
+    // Filter by "c". Folder "b" will also be shown because it is its parent.
+    m_model->setNameFilter("c");
+    QCOMPARE(itemsRemovedSpy.count(), 1); // nothing was removed, itemsRemovedSpy count will remain the same:
     QCOMPARE(m_model->count(), 3);
     QCOMPARE(itemsInModel(), QStringList() << "a" << "b" << "c");
 
@@ -1349,21 +1362,22 @@ void KFileItemModelTest::testGeneralParentChildRelationships()
     m_model->slotCompleted();
     QCOMPARE(itemsInModel(), QStringList() << "parent1" << "realChild1" << "grandChild1" << "realGrandChild1" << "child1" << "parent2" << "realChild2" << "realGrandChild2" << "child2");
 
-    m_model->slotItemsAdded(realChild1, KFileItemList() << KFileItem(QUrl("grandChild1"), QString(), KFileItem::Unknown));
-    m_model->slotCompleted();
-    QCOMPARE(itemsInModel(), QStringList() << "parent1" << "realChild1" << "grandChild1" << "realGrandChild1" << "child1" << "parent2" << "realChild2" << "realGrandChild2" << "child2");
-
     m_model->slotItemsAdded(realChild2, KFileItemList() << KFileItem(QUrl("grandChild2"), QString(), KFileItem::Unknown));
     m_model->slotCompleted();
     QCOMPARE(itemsInModel(), QStringList() << "parent1" << "realChild1" << "grandChild1" << "realGrandChild1" << "child1" << "parent2" << "realChild2" << "grandChild2" << "realGrandChild2" << "child2");
 
-    // Set a name filter that matches nothing -> only expanded folders remain.
+    // Set a name filter that matches nothing -> nothing will remain.
     m_model->setNameFilter("xyz");
-    QCOMPARE(itemsInModel(), QStringList() << "parent1" << "realChild1" << "parent2" << "realChild2");
+    QCOMPARE(itemsInModel(), QStringList());
     QCOMPARE(itemsRemovedSpy.count(), 1);
     QList<QVariant> arguments = itemsRemovedSpy.takeFirst();
     KItemRangeList itemRangeList = arguments.at(0).value<KItemRangeList>();
-    QCOMPARE(itemRangeList, KItemRangeList() << KItemRange(2, 3) << KItemRange(7, 3));
+    QCOMPARE(itemRangeList, KItemRangeList() << KItemRange(0, 10));
+
+    // Set a name filter that matches only "realChild". Their prarents should still show.
+    m_model->setNameFilter("realChild");
+    QCOMPARE(itemsInModel(), QStringList() << "parent1" << "realChild1" << "parent2" << "realChild2");
+    QCOMPARE(itemsRemovedSpy.count(), 0);  // nothing was removed, itemsRemovedSpy will not be called this time
 
     // Collapse "parent1".
     m_model->setExpanded(0, false);