Bug 1649421 - Handle null mimeInfo from a download when populating the context menu. r=jaws

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

browser/components/downloads/DownloadsSubview.jsm

@@ -298,9 +298,8 @@ class DownloadsSubview extends DownloadsViewUI.BaseView {
       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")) {

browser/components/downloads/DownloadsViewUI.jsm

@@ -842,6 +842,11 @@ DownloadsViewUI.DownloadElementShell.prototype = {
     // 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(

browser/components/downloads/content/allDownloadsView.js

@@ -705,9 +705,9 @@ DownloadsPlacesView.prototype = {
     // 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)

browser/components/downloads/content/downloads.js

@@ -938,9 +938,8 @@ var DownloadsView = {
     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");

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

@@ -15,6 +15,7 @@ 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/components/downloads/test/browser/browser_downloads_autohide.js

@@ -497,15 +497,6 @@ function promiseCustomizeEnd(aWindow = window) {
   });
 }
 
-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") {

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);
+}

browser/components/downloads/test/browser/browser_pdfjs_preview.js

@@ -1,9 +1,6 @@
 /* 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(
@@ -368,23 +365,6 @@ function contentTriggerDblclickOn(selector, eventModifiers = {}, browser) {
   );
 }
 
-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(
@@ -393,12 +373,14 @@ async function verifyContextMenu(contextMenu, expected = {}) {
   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,
@@ -428,22 +410,13 @@ async function verifyContextMenu(contextMenu, expected = {}) {
   }
 }
 
-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);
@@ -465,6 +438,7 @@ async function addPDFDownload(itemData) {
     hasPartialData: false,
     hasBlockedData: itemData.hasBlockedData || false,
     startTime: new Date(startTimeMs++),
+    ...itemData,
   };
   if (itemData.errorObj) {
     download.errorObj = itemData.errorObj;
@@ -499,6 +473,7 @@ async function openDownloadPanel(expectedItemCount) {
 async function testOpenPDFPreview({
   name,
   whichUI,
+  downloadProperties,
   itemSelector,
   expected,
   prefs = [],
@@ -517,8 +492,13 @@ async function testOpenPDFPreview({
 
   // 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);

browser/components/downloads/test/browser/head.js

@@ -43,8 +43,41 @@ 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);
@@ -89,13 +122,21 @@ async function task_addDownloads(aItems) {
 
   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 ||
@@ -106,13 +147,16 @@ async function task_addDownloads(aItems) {
           : 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();
   }
 }