Bug 1684524 - Selecting all downloads after a search should not select excluded downloads. r=Gijs

Differential Revision: https://phabricator.services.mozilla.com/D101447

browser/components/downloads/content/allDownloadsView.js

@@ -397,6 +397,10 @@ DownloadsPlacesView.prototype = {
   },
   set searchTerm(aValue) {
     if (this._searchTerm != aValue) {
+      // Always clear selection on a new search, since the user is starting a
+      // different workflow. This also solves the fact we could end up
+      // retaining selection on hidden elements.
+      this._richlistbox.clearSelection();
       for (let element of this._richlistbox.childNodes) {
         element.hidden = !element._shell.matchesSearchTerm(aValue);
       }
@@ -678,7 +682,23 @@ DownloadsPlacesView.prototype = {
   },
 
   cmd_selectAll() {
-    this._richlistbox.selectAll();
+    if (!this.searchTerm) {
+      this._richlistbox.selectAll();
+      return;
+    }
+    // If there is a filtering search term, some rows are hidden and should not
+    // be selected.
+    let oldSuppressOnSelect = this._richlistbox.suppressOnSelect;
+    this._richlistbox.suppressOnSelect = true;
+    this._richlistbox.clearSelection();
+    var item = this._richlistbox.getItemAtIndex(0);
+    while (item) {
+      if (!item.hidden) {
+        this._richlistbox.addItemToSelection(item);
+      }
+      item = this._richlistbox.getNextItem(item, 1);
+    }
+    this._richlistbox.suppressOnSelect = oldSuppressOnSelect;
   },
 
   cmd_paste() {

browser/components/downloads/test/browser/browser.ini

@@ -15,6 +15,7 @@ support-files =
   not-really-a-jpeg.jpeg
   not-really-a-jpeg.jpeg^headers^
   blank.JPG
+[browser_library_select_all.js]
 [browser_overflow_anchor.js]
 skip-if = os == "linux" # Bug 952422
 [browser_confirm_unblock_download.js]

browser/components/downloads/test/browser/browser_library_select_all.js

@@ -0,0 +1,77 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+let gDownloadDir;
+
+add_task(async function setup() {
+  await task_resetState();
+
+  if (!gDownloadDir) {
+    gDownloadDir = await setDownloadDir();
+  }
+
+  await task_addDownloads([
+    {
+      state: DownloadsCommon.DOWNLOAD_FINISHED,
+      target: await createDownloadedFile(
+        PathUtils.join(gDownloadDir, "downloaded_one.txt"),
+        "Test file 1"
+      ),
+    },
+    {
+      state: DownloadsCommon.DOWNLOAD_FINISHED,
+      target: await createDownloadedFile(
+        PathUtils.join(gDownloadDir, "downloaded_two.txt"),
+        "Test file 2"
+      ),
+    },
+  ]);
+  registerCleanupFunction(async function() {
+    await task_resetState();
+    await PlacesUtils.history.clear();
+  });
+});
+
+add_task(async function test_select_all() {
+  let win = await openLibrary("Downloads");
+  registerCleanupFunction(() => {
+    win.close();
+  });
+
+  let listbox = win.document.getElementById("downloadsRichListBox");
+  Assert.ok(listbox, "download list box present");
+  listbox.focus();
+  await TestUtils.waitForCondition(
+    () => listbox.children.length == 2 && listbox.selectedItems.length == 1,
+    "waiting for both items to be present with one selected"
+  );
+  info("Select all the downloads");
+  win.goDoCommand("cmd_selectAll");
+  Assert.equal(
+    listbox.selectedItems.length,
+    listbox.children.length,
+    "All the items should be selected"
+  );
+
+  info("Search for a specific download");
+  let searchBox = win.document.getElementById("searchFilter");
+  searchBox.value = "_one";
+  win.PlacesSearchBox.search(searchBox.value);
+  await TestUtils.waitForCondition(() => {
+    let visibleItems = Array.from(listbox.children).filter(c => !c.hidden);
+    return (
+      visibleItems.length == 1 &&
+      visibleItems[0]._shell.download.target.path.includes("_one")
+    );
+  }, "Waiting for the search to complete");
+  Assert.equal(
+    listbox.selectedItems.length,
+    0,
+    "Check previous selection has been cleared by the search"
+  );
+  info("Select all the downloads");
+  win.goDoCommand("cmd_selectAll");
+  Assert.equal(listbox.children.length, 2, "Both items are present");
+  Assert.equal(listbox.selectedItems.length, 1, "Only one item is selected");
+  Assert.ok(!listbox.selectedItem.hidden, "The selected item is not hidden");
+});