Bug 1642431 - Default to using a new tab when opening downloaded PDFs in the browser. r=jaws
authorSam Foster <sfoster@mozilla.com>
Thu, 18 Jun 2020 15:39:37 +0000
changeset 536324 1f5f8907c765e1905892f71be94978f3f78580ac
parent 536323 63a4089362fcee1e8ebd6507523bd525fc1dc2f1
child 536325 0952f7dec63d88752aa7d52ba8b0da098895316b
push id37520
push userdluca@mozilla.com
push dateFri, 19 Jun 2020 04:04:08 +0000
treeherdermozilla-central@d1a4f9157858 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws
bugs1642431
milestone79.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1642431 - Default to using a new tab when opening downloaded PDFs in the browser. r=jaws 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
@@ -536,17 +536,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;
     }
   }