Bug 1642431 - Default to using a new tab when opening downloaded PDFs in the browser. r=jaws, a=jcristau
authorSam Foster <sfoster@mozilla.com>
Thu, 18 Jun 2020 15:39:37 +0000
changeset 597240 7eb5227276c0b73611489dbad29a9b60ab0af749
parent 597239 c5a047653451b4fcf62918989ac0bfa6ab2c079d
child 597241 5f0dcca70da9f393f3800763306dc8f07821e84a
push id13308
push userjcristau@mozilla.com
push dateMon, 22 Jun 2020 12:38:06 +0000
treeherdermozilla-beta@754df136ed1c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws, jcristau
bugs1642431
milestone78.0
Bug 1642431 - Default to using a new tab when opening downloaded PDFs in the browser. r=jaws, a=jcristau Differential Revision: https://phabricator.services.mozilla.com/D79951
browser/components/downloads/DownloadsCommon.jsm
browser/components/downloads/DownloadsSubview.jsm
browser/components/downloads/DownloadsViewUI.jsm
browser/components/downloads/test/browser/browser_pdfjs_preview.js
browser/components/newtab/lib/DownloadsManager.jsm
--- a/browser/components/downloads/DownloadsCommon.jsm
+++ b/browser/components/downloads/DownloadsCommon.jsm
@@ -428,17 +428,17 @@ var DownloadsCommon = {
 
   /**
    * Opens a downloaded file.
    *
    * @param downloadProperties
    *        A Download object or the initial properties of a serialized download
    * @param options.openWhere
    *        Optional string indicating how to handle opening a download target file URI.
-   *        One of "current", "window", "tab", "tabshifted".
+   *        One of "window", "tab", "tabshifted".
    * @return {Promise}
    * @resolves When the instruction to launch the file has been
    *           successfully given to the operating system or handled internally
    * @rejects  JavaScript exception if there was an error trying to launch
    *           the file.
    */
   async openDownload(download, options) {
     // some download objects got serialized and need reconstituting
--- a/browser/components/downloads/DownloadsSubview.jsm
+++ b/browser/components/downloads/DownloadsSubview.jsm
@@ -381,17 +381,17 @@ class DownloadsSubview extends Downloads
     );
     if (!button || button.localName == "panelview") {
       return;
     }
 
     let item = button.closest(".subviewbutton.download");
 
     let command = "downloadsCmd_open";
-    let openWhere = "current";
+    let openWhere;
     if (button.classList.contains("action-button")) {
       command = item.hasAttribute("canShow")
         ? "downloadsCmd_show"
         : "downloadsCmd_retry";
     } else if (button.localName == "menuitem") {
       command = button.getAttribute("command");
       if (command == "downloadsCmd_clearDownloads") {
         DownloadsSubview.onClearDownloads(button);
--- a/browser/components/downloads/DownloadsViewUI.jsm
+++ b/browser/components/downloads/DownloadsViewUI.jsm
@@ -750,17 +750,17 @@ DownloadsViewUI.DownloadElementShell.pro
     this.download.cancel().catch(() => {});
     this.download.removePartialData().catch(Cu.reportError);
   },
 
   downloadsCmd_confirmBlock() {
     this.download.confirmBlock().catch(Cu.reportError);
   },
 
-  downloadsCmd_open(openWhere = "current") {
+  downloadsCmd_open(openWhere = "tab") {
     DownloadsCommon.openDownload(this.download, {
       openWhere,
     });
   },
 
   downloadsCmd_openReferrer() {
     this.element.ownerGlobal.openURL(
       this.download.source.referrerInfo.originalReferrer
--- a/browser/components/downloads/test/browser/browser_pdfjs_preview.js
+++ b/browser/components/downloads/test/browser/browser_pdfjs_preview.js
@@ -20,32 +20,32 @@ const TestCases = [
     whichUI: "downloadPanel",
     itemSelector: "#downloadsListBox richlistitem .downloadMainArea",
     async userEvents(itemTarget, win) {
       EventUtils.synthesizeMouseAtCenter(itemTarget, {}, win);
     },
     expected: {
       downloadCount: 1,
       newWindow: false,
-      opensTab: false,
+      opensTab: true,
       tabSelected: true,
     },
   },
   {
     name: "Download panel, open from keyboard",
     whichUI: "downloadPanel",
     itemSelector: "#downloadsListBox richlistitem .downloadMainArea",
     async userEvents(itemTarget, win) {
       itemTarget.focus();
       EventUtils.synthesizeKey("VK_RETURN", {}, win);
     },
     expected: {
       downloadCount: 1,
       newWindow: false,
-      opensTab: false,
+      opensTab: true,
       tabSelected: true,
     },
   },
   {
     name: "Download panel, open in new window",
     whichUI: "downloadPanel",
     itemSelector: "#downloadsListBox richlistitem .downloadMainArea",
     async userEvents(itemTarget, win) {
@@ -54,17 +54,17 @@ const TestCases = [
     expected: {
       downloadCount: 1,
       newWindow: true,
       opensTab: false,
       tabSelected: true,
     },
   },
   {
-    name: "Download panel, open foreground tab",
+    name: "Download panel, open foreground tab", // duplicates the default behavior
     whichUI: "downloadPanel",
     itemSelector: "#downloadsListBox richlistitem .downloadMainArea",
     async userEvents(itemTarget, win) {
       EventUtils.synthesizeMouseAtCenter(
         itemTarget,
         { ctrlKey: true, metaKey: true },
         win
       );
@@ -100,31 +100,31 @@ const TestCases = [
     whichUI: "allDownloads",
     async userEvents(itemTarget, win) {
       // double click
       await triggerDblclickOn(itemTarget, {}, win);
     },
     expected: {
       downloadCount: 1,
       newWindow: false,
-      opensTab: false,
+      opensTab: true,
       tabSelected: true,
     },
   },
   {
     name: "Library all downloads dialog, open from keyboard",
     whichUI: "allDownloads",
     async userEvents(itemTarget, win) {
       itemTarget.focus();
       EventUtils.synthesizeKey("VK_RETURN", {}, win);
     },
     expected: {
       downloadCount: 1,
       newWindow: false,
-      opensTab: false,
+      opensTab: true,
       tabSelected: true,
     },
   },
   {
     name: "Library all downloads dialog, open in new window",
     whichUI: "allDownloads",
     async userEvents(itemTarget, win) {
       // double click
@@ -133,17 +133,17 @@ const TestCases = [
     expected: {
       downloadCount: 1,
       newWindow: true,
       opensTab: false,
       tabSelected: true,
     },
   },
   {
-    name: "Library all downloads dialog, open foreground tab",
+    name: "Library all downloads dialog, open foreground tab", // duplicates default behavior
     whichUI: "allDownloads",
     async userEvents(itemTarget, win) {
       // double click
       await triggerDblclickOn(
         itemTarget,
         { ctrlKey: true, metaKey: true },
         win
       );
@@ -180,17 +180,17 @@ const TestCases = [
     async userEvents(itemSelector, win) {
       let browser = win.gBrowser.selectedBrowser;
       is(browser.currentURI.spec, "about:downloads");
       await contentTriggerDblclickOn(itemSelector, {}, browser);
     },
     expected: {
       downloadCount: 1,
       newWindow: false,
-      opensTab: false,
+      opensTab: true,
       tabSelected: true,
     },
   },
   {
     name: "about:downloads, open in new window",
     whichUI: "aboutDownloads",
     itemSelector: "#downloadsRichListBox richlistitem .downloadContainer",
     async userEvents(itemSelector, win) {
--- a/browser/components/newtab/lib/DownloadsManager.jsm
+++ b/browser/components/newtab/lib/DownloadsManager.jsm
@@ -168,22 +168,25 @@ this.DownloadsManager = class DownloadsM
         doDownloadAction(download => {
           DownloadsCommon.showDownloadedFile(
             new FileUtils.File(download.target.path)
           );
         });
         break;
       case at.OPEN_DOWNLOAD_FILE:
         const win = action._target.browser.ownerGlobal;
-        const openWhere = action.data.event
-          ? win.whereToOpenLink(action.data.event)
-          : "current";
+        const openWhere =
+          action.data.event && win.whereToOpenLink(action.data.event);
         doDownloadAction(download => {
           DownloadsCommon.openDownload(download, {
-            openWhere,
+            // Replace "current" or unknown value with "tab" as the default behavior
+            // for opening downloads when handled internally
+            openWhere: ["window", "tab", "tabshifted"].includes(openWhere)
+              ? openWhere
+              : "tab",
           });
         });
         break;
       case at.UNINIT:
         this.uninit();
         break;
     }
   }