Bug 1649421 - Handle null mimeInfo from a download when populating the context menu. r=jaws, a=RyanVM
authorSam Foster <sfoster@mozilla.com>
Wed, 01 Jul 2020 22:27:51 +0000
changeset 601902 c5afbc5bd6936b4e3aad96b8d8d4ff2ec17f7a85
parent 601901 2baeb9bff2ee062cd8cfdd0e05497c105ec15363
child 601903 51bedc350693e96f61507ed3c79ec4f230a1adc1
push id13344
push userryanvm@gmail.com
push dateFri, 03 Jul 2020 00:16:09 +0000
treeherdermozilla-beta@51bedc350693 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws, RyanVM
bugs1649421
milestone79.0
Bug 1649421 - Handle null mimeInfo from a download when populating the context menu. r=jaws, a=RyanVM Differential Revision: https://phabricator.services.mozilla.com/D81767
browser/components/downloads/DownloadsSubview.jsm
browser/components/downloads/DownloadsViewUI.jsm
browser/components/downloads/content/allDownloadsView.js
browser/components/downloads/content/downloads.js
browser/components/downloads/test/browser/browser.ini
browser/components/downloads/test/browser/browser_downloads_autohide.js
browser/components/downloads/test/browser/browser_downloads_panel_context_menu.js
browser/components/downloads/test/browser/browser_pdfjs_preview.js
browser/components/downloads/test/browser/head.js
--- a/browser/components/downloads/DownloadsSubview.jsm
+++ b/browser/components/downloads/DownloadsSubview.jsm
@@ -293,19 +293,18 @@ class DownloadsSubview extends Downloads
    * @param {DOMNode} button The Button the context menu will be anchored to.
    * @param {DOMNode} menu   The context menu.
    */
   static updateContextMenu(button, menu) {
     while (!button._shell) {
       button = button.parentNode;
     }
     let download = button._shell.download;
-    let { preferredAction, useSystemDefault } = DownloadsCommon.getMimeInfo(
-      download
-    );
+    let mimeInfo = DownloadsCommon.getMimeInfo(download);
+    let { preferredAction, useSystemDefault } = mimeInfo ? mimeInfo : {};
 
     menu.setAttribute("state", button.getAttribute("state"));
     if (button.hasAttribute("exists")) {
       menu.setAttribute("exists", button.getAttribute("exists"));
     } else {
       menu.removeAttribute("exists");
     }
     menu.classList.toggle(
--- a/browser/components/downloads/DownloadsViewUI.jsm
+++ b/browser/components/downloads/DownloadsViewUI.jsm
@@ -837,16 +837,21 @@ DownloadsViewUI.DownloadElementShell.pro
       useSystemDefault: true,
     }).catch(Cu.reportError);
   },
 
   downloadsCmd_alwaysOpenInSystemViewer() {
     // this command toggles between setting preferredAction for this mime-type to open
     // using the system viewer, or to open the file in browser.
     const mimeInfo = DownloadsCommon.getMimeInfo(this.download);
+    if (!mimeInfo) {
+      throw new Error(
+        "Can't open download with unknown mime-type in system viewer"
+      );
+    }
     if (mimeInfo.preferredAction !== mimeInfo.useSystemDefault) {
       // User has selected to open this mime-type with the system viewer from now on
       DownloadsCommon.log(
         "downloadsCmd_alwaysOpenInSystemViewer command for download: ",
         this.download,
         "switching to use system default for " + mimeInfo.type
       );
       mimeInfo.preferredAction = mimeInfo.useSystemDefault;
--- a/browser/components/downloads/content/allDownloadsView.js
+++ b/browser/components/downloads/content/allDownloadsView.js
@@ -700,19 +700,19 @@ DownloadsPlacesView.prototype = {
     let element = this._richlistbox.selectedItem;
     if (!element || !element._shell) {
       return false;
     }
 
     // Set the state attribute so that only the appropriate items are displayed.
     let contextMenu = document.getElementById("downloadsContextMenu");
     let download = element._shell.download;
-    let { preferredAction, useSystemDefault } = DownloadsCommon.getMimeInfo(
-      download
-    );
+    let mimeInfo = DownloadsCommon.getMimeInfo(download);
+    let { preferredAction, useSystemDefault } = mimeInfo ? mimeInfo : {};
+
     contextMenu.setAttribute(
       "state",
       DownloadsCommon.stateOfDownload(download)
     );
     contextMenu.setAttribute("exists", "true");
     contextMenu.classList.toggle("temporary-block", !!download.hasBlockedData);
 
     if (element.hasAttribute("is-pdf")) {
--- a/browser/components/downloads/content/downloads.js
+++ b/browser/components/downloads/content/downloads.js
@@ -933,19 +933,18 @@ var DownloadsView = {
     let element = this.richListBox.selectedItem;
     if (!element) {
       return;
     }
 
     DownloadsViewController.updateCommands();
 
     let download = element._shell.download;
-    let { preferredAction, useSystemDefault } = DownloadsCommon.getMimeInfo(
-      download
-    );
+    let mimeInfo = DownloadsCommon.getMimeInfo(download);
+    let { preferredAction, useSystemDefault } = mimeInfo ? mimeInfo : {};
 
     // Set the state attribute so that only the appropriate items are displayed.
     let contextMenu = document.getElementById("downloadsContextMenu");
     contextMenu.setAttribute("state", element.getAttribute("state"));
     if (element.hasAttribute("exists")) {
       contextMenu.setAttribute("exists", "true");
     } else {
       contextMenu.removeAttribute("exists");
--- a/browser/components/downloads/test/browser/browser.ini
+++ b/browser/components/downloads/test/browser/browser.ini
@@ -10,13 +10,14 @@ skip-if = os == "linux" # Bug 952422
 [browser_confirm_unblock_download.js]
 [browser_iframe_gone_mid_download.js]
 [browser_indicatorDrop.js]
 [browser_libraryDrop.js]
 skip-if = (os == 'win' && os_version == '10.0' && ccov) # Bug 1306510
 [browser_library_clearall.js]
 [browser_downloads_panel_block.js]
 skip-if = true # Bug 1352792
+[browser_downloads_panel_context_menu.js]
 [browser_downloads_panel_ctrl_click.js]
 [browser_downloads_panel_height.js]
 [browser_downloads_autohide.js]
 [browser_go_to_download_page.js]
 [browser_pdfjs_preview.js]
\ No newline at end of file
--- a/browser/components/downloads/test/browser/browser_downloads_autohide.js
+++ b/browser/components/downloads/test/browser/browser_downloads_autohide.js
@@ -492,25 +492,16 @@ function promiseCustomizeEnd(aWindow = w
   return new Promise(resolve => {
     aWindow.gNavToolbox.addEventListener("aftercustomization", resolve, {
       once: true,
     });
     aWindow.gCustomizeMode.exit();
   });
 }
 
-async function openContextMenu(element) {
-  let popupShownPromise = BrowserTestUtils.waitForEvent(document, "popupshown");
-  EventUtils.synthesizeMouseAtCenter(element, {
-    type: "contextmenu",
-    button: 2,
-  });
-  await popupShownPromise;
-}
-
 function clickCheckbox(checkbox) {
   // Clicking a checkbox toggles its checkedness first.
   if (checkbox.getAttribute("checked") == "true") {
     checkbox.removeAttribute("checked");
   } else {
     checkbox.setAttribute("checked", "true");
   }
   // Then it runs the command and closes the popup.
new file mode 100644
--- /dev/null
+++ b/browser/components/downloads/test/browser/browser_downloads_panel_context_menu.js
@@ -0,0 +1,204 @@
+/*
+  Coverage for context menu state for downloads in the Downloads Panel
+*/
+
+let gDownloadDir;
+const TestFiles = {};
+
+const MENU_ITEMS = {
+  pause: ".downloadPauseMenuItem",
+  resume: ".downloadResumeMenuItem",
+  unblock: '[command="downloadsCmd_unblock"]',
+  openInSystemViewer: '[command="downloadsCmd_openInSystemViewer"]',
+  alwaysOpenInSystemViewer: '[command="downloadsCmd_alwaysOpenInSystemViewer"]',
+  show: '[command="downloadsCmd_show"]',
+  commandsSeparator: "menuseparator,.downloadCommandsSeparator",
+  openReferrer: '[command="downloadsCmd_openReferrer"]',
+  copyLocation: '[command="downloadsCmd_copyLocation"]',
+  separator: "menuseparator",
+  delete: '[command="cmd_delete"]',
+  clearList: '[command="downloadsCmd_clearList"]',
+  clearDownloads: '[command="downloadsCmd_clearDownloads"]',
+};
+
+const TestCases = [
+  {
+    name: "Completed PDF download",
+    downloads: [
+      {
+        state: DownloadsCommon.DOWNLOAD_FINISHED,
+        contentType: "application/pdf",
+        target: {},
+      },
+    ],
+    expected: {
+      menu: [
+        MENU_ITEMS.openInSystemViewer,
+        MENU_ITEMS.alwaysOpenInSystemViewer,
+        MENU_ITEMS.show,
+        MENU_ITEMS.commandsSeparator,
+        MENU_ITEMS.openReferrer,
+        MENU_ITEMS.copyLocation,
+        MENU_ITEMS.separator,
+        MENU_ITEMS.delete,
+        MENU_ITEMS.clearList,
+      ],
+    },
+  },
+  {
+    name: "Canceled PDF download",
+    downloads: [
+      {
+        state: DownloadsCommon.DOWNLOAD_CANCELED,
+        contentType: "application/pdf",
+        target: {},
+      },
+    ],
+    expected: {
+      menu: [
+        MENU_ITEMS.openReferrer,
+        MENU_ITEMS.copyLocation,
+        MENU_ITEMS.separator,
+        MENU_ITEMS.delete,
+        MENU_ITEMS.clearList,
+      ],
+    },
+  },
+];
+
+add_task(async function test_setUp() {
+  // remove download files, empty out collections
+  let downloadList = await Downloads.getList(Downloads.ALL);
+  let downloadCount = (await downloadList.getAll()).length;
+  is(downloadCount, 0, "At the start of the test, there should be 0 downloads");
+
+  await task_resetState();
+  if (!gDownloadDir) {
+    gDownloadDir = await setDownloadDir();
+  }
+  info("Created download directory: " + gDownloadDir);
+
+  // create the downloaded files we'll need
+  TestFiles.pdf = await createDownloadedFile(
+    OS.Path.join(gDownloadDir, "downloaded.pdf"),
+    DATA_PDF
+  );
+  info("Created downloaded PDF file at:" + TestFiles.pdf.path);
+  TestFiles.txt = await createDownloadedFile(
+    OS.Path.join(gDownloadDir, "downloaded.txt"),
+    "Test file"
+  );
+  info("Created downloaded text file at:" + TestFiles.txt.path);
+});
+
+// register the tests
+for (let testData of TestCases) {
+  if (testData.skip) {
+    info("Skipping test:" + testData.name);
+    continue;
+  }
+  // use the 'name' property of each test case as the test function name
+  // so we get useful logs
+  let tmp = {
+    async [testData.name]() {
+      await testDownloadContextMenu(testData);
+    },
+  };
+  add_task(tmp[testData.name]);
+}
+
+async function testDownloadContextMenu({ downloads = [], expected }) {
+  // prepare downloads
+  await prepareDownloads(downloads);
+  let downloadList = await Downloads.getList(Downloads.PUBLIC);
+  let [firstDownload] = await downloadList.getAll();
+  info("Download succeeded? " + firstDownload.succeeded);
+  info("Download target exists? " + firstDownload.target.exists);
+
+  // open panel
+  await task_openPanel();
+  await TestUtils.waitForCondition(
+    () =>
+      document.getElementById("downloadsListBox").childElementCount ==
+      downloads.length
+  );
+
+  info("trigger the context menu");
+  let itemTarget = document.querySelector(
+    "#downloadsListBox richlistitem .downloadMainArea"
+  );
+
+  let contextMenu = await openContextMenu(itemTarget);
+
+  info("context menu should be open, verify its menu items");
+  let result = verifyContextMenu(contextMenu, expected.menu);
+
+  // close menus
+  contextMenu.hidePopup();
+  let hiddenPromise = BrowserTestUtils.waitForEvent(
+    DownloadsPanel.panel,
+    "popuphidden"
+  );
+  DownloadsPanel.hidePanel();
+  await hiddenPromise;
+
+  ok(!result, "Expected no errors verifying context menu items");
+
+  // clean up downloads
+  await downloadList.removeFinished();
+}
+
+// ----------------------------------------------------------------------------
+// Helpers
+
+function verifyContextMenu(contextMenu, itemSelectors) {
+  // Ignore hidden nodes
+  let items = Array.from(contextMenu.children).filter(n =>
+    BrowserTestUtils.is_visible(n)
+  );
+  let menuAsText = items
+    .map(n => {
+      return n.nodeName == "menuseparator"
+        ? "---"
+        : `${n.label} (${n.command})`;
+    })
+    .join("\n");
+  info("Got actual context menu items: \n" + menuAsText);
+
+  try {
+    is(
+      items.length,
+      itemSelectors.length,
+      "Context menu has the expected number of items"
+    );
+    for (let i = 0; i < items.length; i++) {
+      let selector = itemSelectors[i];
+      ok(
+        items[i].matches(selector),
+        `Item at ${i} matches expected selector: ${selector}`
+      );
+    }
+  } catch (ex) {
+    return ex;
+  }
+  return null;
+}
+
+async function prepareDownloads(downloads) {
+  for (let props of downloads) {
+    info(JSON.stringify(props));
+    if (props.state !== DownloadsCommon.DOWNLOAD_FINISHED) {
+      continue;
+    }
+    switch (props.contentType) {
+      case "application/pdf":
+        props.target = TestFiles.pdf;
+        break;
+      default:
+        props.target = TestFiles.txt;
+        break;
+    }
+    ok(props.target instanceof Ci.nsIFile, "download target is a nsIFile");
+  }
+  await task_addDownloads(downloads);
+}
--- a/browser/components/downloads/test/browser/browser_pdfjs_preview.js
+++ b/browser/components/downloads/test/browser/browser_pdfjs_preview.js
@@ -1,14 +1,11 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
-const DATA_PDF = atob(
-  "JVBERi0xLjANCjEgMCBvYmo8PC9UeXBlL0NhdGFsb2cvUGFnZXMgMiAwIFI+PmVuZG9iaiAyIDAgb2JqPDwvVHlwZS9QYWdlcy9LaWRzWzMgMCBSXS9Db3VudCAxPj5lbmRvYmogMyAwIG9iajw8L1R5cGUvUGFnZS9NZWRpYUJveFswIDAgMyAzXT4+ZW5kb2JqDQp4cmVmDQowIDQNCjAwMDAwMDAwMDAgNjU1MzUgZg0KMDAwMDAwMDAxMCAwMDAwMCBuDQowMDAwMDAwMDUzIDAwMDAwIG4NCjAwMDAwMDAxMDIgMDAwMDAgbg0KdHJhaWxlcjw8L1NpemUgNC9Sb290IDEgMCBSPj4NCnN0YXJ0eHJlZg0KMTQ5DQolRU9G"
-);
 let gDownloadDir;
 
 SimpleTest.requestFlakyTimeout(
   "Giving a chance for possible last-pb-context-exited to occur (Bug 1329912)"
 );
 
 /*
   Coverage for opening downloaded PDFs from download views
@@ -363,47 +360,32 @@ function contentTriggerDblclickOn(select
         content
       );
       info("Waiting for double-click content task");
       await doubleClicked;
     }
   );
 }
 
-async function openContextMenu(itemElement, win = window) {
-  let popupShownPromise = BrowserTestUtils.waitForEvent(
-    itemElement.ownerDocument,
-    "popupshown"
-  );
-  EventUtils.synthesizeMouseAtCenter(
-    itemElement,
-    {
-      type: "contextmenu",
-      button: 2,
-    },
-    win
-  );
-  let { target } = await popupShownPromise;
-  return target;
-}
-
 async function verifyContextMenu(contextMenu, expected = {}) {
   info("verifyContextMenu with expected: " + JSON.stringify(expected, null, 2));
   let alwaysMenuItem = contextMenu.querySelector(
     ".downloadAlwaysUseSystemDefaultMenuItem"
   );
   let useSystemMenuItem = contextMenu.querySelector(
     ".downloadUseSystemDefaultMenuItem"
   );
+  info("Waiting for the context menu to show up");
   await TestUtils.waitForCondition(
     () => BrowserTestUtils.is_visible(contextMenu),
     "The context menu is visible"
   );
   await TestUtils.waitForTick();
 
+  info("Checking visibility of the system viewer menu items");
   is(
     BrowserTestUtils.is_hidden(useSystemMenuItem),
     expected.useSystemMenuItemDisabled,
     `The 'Use system viewer' menu item was ${
       expected.useSystemMenuItemDisabled ? "hidden" : "visible"
     }`
   );
   is(
@@ -423,32 +405,23 @@ async function verifyContextMenu(context
   } else if (!expected.useSystemMenuItemDisabled) {
     ok(
       !alwaysMenuItem.hasAttribute("checked"),
       "The 'Always...' menu item not checked"
     );
   }
 }
 
-async function createDownloadedFile(pathname, contents) {
-  let encoder = new TextEncoder();
-  let file = new FileUtils.File(pathname);
-  if (file.exists()) {
-    info(`File at ${pathname} already exists`);
-  }
-  // No post-test cleanup necessary; tmp downloads directory is already removed after each test
-  await OS.File.writeAtomic(pathname, encoder.encode(contents));
-  ok(file.exists(), `Created ${pathname}`);
-  return file;
-}
-
 async function addPDFDownload(itemData) {
   let startTimeMs = Date.now();
+  info("addPDFDownload with itemData: " + JSON.stringify(itemData, null, 2));
 
   let downloadPathname = OS.Path.join(gDownloadDir, itemData.targetFilename);
+  delete itemData.targetFilename;
+
   info("Creating saved download file at:" + downloadPathname);
   let pdfFile = await createDownloadedFile(downloadPathname, DATA_PDF);
   info("Created file at:" + pdfFile.path);
 
   let downloadList = await Downloads.getList(
     itemData.isPrivate ? Downloads.PRIVATE : Downloads.PUBLIC
   );
   let download = {
@@ -460,16 +433,17 @@ async function addPDFDownload(itemData) 
       path: pdfFile.path,
     },
     succeeded: DownloadsCommon.DOWNLOAD_FINISHED,
     canceled: false,
     error: null,
     hasPartialData: false,
     hasBlockedData: itemData.hasBlockedData || false,
     startTime: new Date(startTimeMs++),
+    ...itemData,
   };
   if (itemData.errorObj) {
     download.errorObj = itemData.errorObj;
   }
 
   await downloadList.add(await Downloads.createDownload(download));
   return download;
 }
@@ -494,16 +468,17 @@ async function openDownloadPanel(expecte
   await TestUtils.waitForCondition(
     () => richlistbox.childElementCount == expectedItemCount
   );
 }
 
 async function testOpenPDFPreview({
   name,
   whichUI,
+  downloadProperties,
   itemSelector,
   expected,
   prefs = [],
   userEvents,
   isPrivate,
 }) {
   info("Test case: " + name);
   // Wait for focus first
@@ -512,18 +487,23 @@ async function testOpenPDFPreview({
   if (prefs.length) {
     await SpecialPowers.pushPrefEnv({
       set: prefs,
     });
   }
 
   // Populate downloads database with the data required by this test.
   info("Adding download objects");
+  if (!downloadProperties) {
+    downloadProperties = {
+      targetFilename: "downloaded.pdf",
+    };
+  }
   let download = await addPDFDownload({
-    targetFilename: "downloaded.pdf",
+    ...downloadProperties,
     isPrivate,
   });
   info("Got download pathname:" + download.target.path);
   is(
     !!download.source.isPrivate,
     !!isPrivate,
     `Added download is ${isPrivate ? "private" : "not private"} as expected`
   );
--- a/browser/components/downloads/test/browser/head.js
+++ b/browser/components/downloads/test/browser/head.js
@@ -38,18 +38,51 @@ ChromeUtils.defineModuleGetter(
 var gTestTargetFile = FileUtils.getFile("TmpD", ["dm-ui-test.file"]);
 gTestTargetFile.createUnique(Ci.nsIFile.NORMAL_FILE_TYPE, FileUtils.PERMS_FILE);
 
 // The file may have been already deleted when removing a paused download.
 registerCleanupFunction(() =>
   OS.File.remove(gTestTargetFile.path, { ignoreAbsent: true })
 );
 
+const DATA_PDF = atob(
+  "JVBERi0xLjANCjEgMCBvYmo8PC9UeXBlL0NhdGFsb2cvUGFnZXMgMiAwIFI+PmVuZG9iaiAyIDAgb2JqPDwvVHlwZS9QYWdlcy9LaWRzWzMgMCBSXS9Db3VudCAxPj5lbmRvYmogMyAwIG9iajw8L1R5cGUvUGFnZS9NZWRpYUJveFswIDAgMyAzXT4+ZW5kb2JqDQp4cmVmDQowIDQNCjAwMDAwMDAwMDAgNjU1MzUgZg0KMDAwMDAwMDAxMCAwMDAwMCBuDQowMDAwMDAwMDUzIDAwMDAwIG4NCjAwMDAwMDAxMDIgMDAwMDAgbg0KdHJhaWxlcjw8L1NpemUgNC9Sb290IDEgMCBSPj4NCnN0YXJ0eHJlZg0KMTQ5DQolRU9G"
+);
+
 // Asynchronous support subroutines
 
+async function createDownloadedFile(pathname, contents) {
+  let encoder = new TextEncoder();
+  let file = new FileUtils.File(pathname);
+  if (file.exists()) {
+    info(`File at ${pathname} already exists`);
+  }
+  // No post-test cleanup necessary; tmp downloads directory is already removed after each test
+  await OS.File.writeAtomic(pathname, encoder.encode(contents));
+  ok(file.exists(), `Created ${pathname}`);
+  return file;
+}
+
+async function openContextMenu(itemElement, win = window) {
+  let popupShownPromise = BrowserTestUtils.waitForEvent(
+    itemElement.ownerDocument,
+    "popupshown"
+  );
+  EventUtils.synthesizeMouseAtCenter(
+    itemElement,
+    {
+      type: "contextmenu",
+      button: 2,
+    },
+    win
+  );
+  let { target } = await popupShownPromise;
+  return target;
+}
+
 function promiseFocus() {
   return new Promise(resolve => {
     waitForFocus(resolve);
   });
 }
 
 function promisePanelOpened() {
   if (DownloadsPanel.panel && DownloadsPanel.panel.state == "open") {
@@ -84,40 +117,51 @@ async function task_resetState() {
   await promiseFocus();
 }
 
 async function task_addDownloads(aItems) {
   let startTimeMs = Date.now();
 
   let publicList = await Downloads.getList(Downloads.PUBLIC);
   for (let item of aItems) {
+    let source = {
+      url: "http://www.example.com/test-download.txt",
+      ...item.source,
+    };
+    let target =
+      item.target instanceof Ci.nsIFile
+        ? item.target
+        : {
+            path: gTestTargetFile.path,
+            ...item.target,
+          };
+
     let download = {
-      source: {
-        url: "http://www.example.com/test-download.txt",
-      },
-      target: {
-        path: gTestTargetFile.path,
-      },
+      source,
+      target,
       succeeded: item.state == DownloadsCommon.DOWNLOAD_FINISHED,
       canceled:
         item.state == DownloadsCommon.DOWNLOAD_CANCELED ||
         item.state == DownloadsCommon.DOWNLOAD_PAUSED,
       error:
         item.state == DownloadsCommon.DOWNLOAD_FAILED
           ? new Error("Failed.")
           : null,
       hasPartialData: item.state == DownloadsCommon.DOWNLOAD_PAUSED,
       hasBlockedData: item.hasBlockedData || false,
+      contentType: item.contentType,
       startTime: new Date(startTimeMs++),
     };
     // `"errorObj" in download` must be false when there's no error.
     if (item.errorObj) {
       download.errorObj = item.errorObj;
     }
-    await publicList.add(await Downloads.createDownload(download));
+    download = await Downloads.createDownload(download);
+    await publicList.add(download);
+    await download.refresh();
   }
 }
 
 async function task_openPanel() {
   await promiseFocus();
 
   let promise = promisePanelOpened();
   DownloadsPanel.showPanel();