Backed out changeset a81a827f54fb (bug 1711053) for failures on browser_download_opens_on_click.js. CLOSED TREE
authorCsoregi Natalia <ncsoregi@mozilla.com>
Wed, 07 Jul 2021 13:20:05 +0300
changeset 584968 d2ab10b1b1619f8e2473837ed9043ad28c842aa5
parent 584967 f4dfcf4c7f705d1ad99ba3af3b80cda98141e7c6
child 584969 6b2a1a09cd4735386414483dd4050c25f9e58e86
push id145902
push userncsoregi@mozilla.com
push dateWed, 07 Jul 2021 10:29:25 +0000
treeherderautoland@d2ab10b1b161 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1711053
milestone91.0a1
backs outa81a827f54fb0aa447d8585268d707ab7b57b0b4
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
Backed out changeset a81a827f54fb (bug 1711053) for failures on browser_download_opens_on_click.js. CLOSED TREE
browser/components/downloads/DownloadsViewUI.jsm
browser/components/downloads/content/downloads.js
browser/components/downloads/test/browser/browser.ini
browser/components/downloads/test/browser/browser_download_opens_on_click.js
browser/components/downloads/test/browser/head.js
browser/locales/en-US/browser/downloads.ftl
browser/themes/shared/downloads/downloads.inc.css
toolkit/mozapps/downloads/DownloadUtils.jsm
--- a/browser/components/downloads/DownloadsViewUI.jsm
+++ b/browser/components/downloads/DownloadsViewUI.jsm
@@ -10,18 +10,16 @@
 "use strict";
 
 var EXPORTED_SYMBOLS = ["DownloadsViewUI"];
 
 const { XPCOMUtils } = ChromeUtils.import(
   "resource://gre/modules/XPCOMUtils.jsm"
 );
 
-const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
-
 XPCOMUtils.defineLazyModuleGetters(this, {
   BrowserWindowTracker: "resource:///modules/BrowserWindowTracker.jsm",
   Downloads: "resource://gre/modules/Downloads.jsm",
   DownloadUtils: "resource://gre/modules/DownloadUtils.jsm",
   DownloadsCommon: "resource:///modules/DownloadsCommon.jsm",
   FileUtils: "resource://gre/modules/FileUtils.jsm",
   OS: "resource://gre/modules/osfile.jsm",
 });
@@ -409,39 +407,21 @@ DownloadsViewUI.DownloadElementShell.pro
    *        Status line of the Downloads Panel or the Downloads View.
    * @param hoverStatus
    *        Label to show in the Downloads Panel when the mouse pointer is over
    *        the main area of the item. If not specified, this will be the same
    *        as the status line. This is ignored in the Downloads View. Type is
    *        either l10n object or string literal.
    */
   showStatus(status, hoverStatus = status) {
-    let document = this.element.ownerDocument;
-    if (status?.l10n) {
-      document.l10n.setAttributes(
-        this._downloadDetailsNormal,
-        status.l10n.id,
-        status.l10n.args
-      );
-    } else {
-      this._downloadDetailsNormal.removeAttribute("data-l10n-id");
-      this._downloadDetailsNormal.setAttribute("value", status);
-      this._downloadDetailsNormal.setAttribute("tooltiptext", status);
-    }
-    if (hoverStatus?.l10n) {
-      hoverStatus.l10n.id
-        ? document.l10n.setAttributes(
-            this._downloadDetailsHover,
-            hoverStatus.l10n.id,
-            hoverStatus.l10n.args
-          )
-        : document.l10n.setAttributes(
-            this._downloadDetailsHover,
-            hoverStatus.l10n
-          );
+    this._downloadDetailsNormal.setAttribute("value", status);
+    this._downloadDetailsNormal.setAttribute("tooltiptext", status);
+    if (hoverStatus && hoverStatus.l10n) {
+      let document = this.element.ownerDocument;
+      document.l10n.setAttributes(this._downloadDetailsHover, hoverStatus.l10n);
     } else {
       this._downloadDetailsHover.removeAttribute("data-l10n-id");
       this._downloadDetailsHover.setAttribute("value", hoverStatus);
     }
   },
 
   /**
    * Updates the status line combining the given state label with other labels.
@@ -550,44 +530,30 @@ DownloadsViewUI.DownloadElementShell.pro
    * This is called for all changes in the download, including progress updates.
    * For major state changes, _updateState is called first, but several elements
    * are still updated here. When the download is in progress, this function
    * takes a faster path with less element updates to improve performance.
    */
   _updateStateInner() {
     let progressPaused = false;
 
-    let improvementsOn = Services.prefs.getBoolPref(
-      "browser.download.improvements_to_download_panel"
-    );
-
-    this.element.classList.toggle(
-      "openWhenFinished",
-      improvementsOn && !this.download.stopped
-    );
-
     if (!this.download.stopped) {
       // The download is in progress, so we don't change the button state
       // because the _updateState function already did it. We still need to
       // update all elements that may change during the download.
       let totalBytes = this.download.hasProgress
         ? this.download.totalBytes
         : -1;
       let [status, newEstimatedSecondsLeft] = DownloadUtils.getDownloadStatus(
         this.download.currentBytes,
         totalBytes,
         this.download.speed,
         this.lastEstimatedSecondsLeft
       );
       this.lastEstimatedSecondsLeft = newEstimatedSecondsLeft;
-
-      if (improvementsOn && this.download.launchWhenSucceeded) {
-        status = DownloadUtils.getFormattedTimeStatus(newEstimatedSecondsLeft);
-      }
-
       this.showStatus(status);
     } else {
       let verdict = "";
 
       // The download is not in progress, so we update the user interface based
       // on other properties. The order in which we check the properties of the
       // Download object is the same used by stateOfDownload.
       if (this.download.succeeded) {
--- a/browser/components/downloads/content/downloads.js
+++ b/browser/components/downloads/content/downloads.js
@@ -833,27 +833,16 @@ var DownloadsView = {
       } else if (aEvent.shiftKey || aEvent.ctrlKey || aEvent.metaKey) {
         // We adjust the command for supported modifiers to suggest where the download
         // may be opened
         let openWhere = target.ownerGlobal.whereToOpenLink(aEvent, false, true);
         if (["tab", "window", "tabshifted"].includes(openWhere)) {
           command += ":" + openWhere;
         }
       }
-      // Toggle opening the file after the download has completed
-      if (
-        !download.stopped &&
-        command.startsWith("downloadsCmd_open") &&
-        Services.prefs.getBoolPref(
-          "browser.download.improvements_to_download_panel"
-        )
-      ) {
-        download.launchWhenSucceeded = !download.launchWhenSucceeded;
-      }
-
       DownloadsCommon.log("onDownloadClick, resolved command: ", command);
       goDoCommand(command);
     }
   },
 
   onDownloadButton(event) {
     let target = event.target.closest("richlistitem");
     DownloadsView.itemForElement(target).onButton();
--- a/browser/components/downloads/test/browser/browser.ini
+++ b/browser/components/downloads/test/browser/browser.ini
@@ -19,17 +19,16 @@ support-files =
 [browser_overflow_anchor.js]
 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_download_opens_on_click.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_panel_opens.js]
 [browser_downloads_autohide.js]
 [browser_go_to_download_page.js]
deleted file mode 100644
--- a/browser/components/downloads/test/browser/browser_download_opens_on_click.js
+++ /dev/null
@@ -1,60 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
- * http://creativecommons.org/publicdomain/zero/1.0/ */
-
-ChromeUtils.defineModuleGetter(
-  this,
-  "DownloadsViewUI",
-  "resource:///modules/DownloadsViewUI.jsm"
-);
-
-add_task(async function test_download_clickable() {
-  await SpecialPowers.pushPrefEnv({
-    set: [["browser.download.improvements_to_download_panel", true]],
-  });
-
-  startServer();
-  mustInterruptResponses();
-  let download = await promiseInterruptibleDownload();
-  let publicList = await Downloads.getList(Downloads.PUBLIC);
-  registerCleanupFunction(async function() {
-    continueResponses();
-    task_resetState();
-  });
-  await publicList.add(download);
-  download.start();
-
-  await promiseDownloadHasProgress(download, 50);
-
-  await task_openPanel();
-
-  let listbox = document.getElementById("downloadsListBox");
-  ok(listbox, "Download list box present");
-
-  await TestUtils.waitForCondition(() => {
-    return listbox.childElementCount == 1;
-  });
-
-  info("All downloads show in the listbox.itemChildren ", listbox.itemChildren);
-
-  ok(
-    listbox.itemChildren[0].classList.contains("openWhenFinished"),
-    "Download should have clickable style when in progress"
-  );
-
-  listbox.itemChildren[0].click();
-  ok(
-    download.launchWhenSucceeded,
-    "Should open the file when download is finished"
-  );
-
-  listbox.itemChildren[0].click();
-
-  ok(
-    !download.launchWhenSucceeded,
-    "Should NOT open the file when download is finished"
-  );
-
-  continueResponses();
-  await download.refresh();
-  await promiseDownloadHasProgress(download, 100);
-});
--- a/browser/components/downloads/test/browser/head.js
+++ b/browser/components/downloads/test/browser/head.js
@@ -42,70 +42,16 @@ gTestTargetFile.createUnique(Ci.nsIFile.
 registerCleanupFunction(() =>
   OS.File.remove(gTestTargetFile.path, { ignoreAbsent: true })
 );
 
 const DATA_PDF = atob(
   "JVBERi0xLjANCjEgMCBvYmo8PC9UeXBlL0NhdGFsb2cvUGFnZXMgMiAwIFI+PmVuZG9iaiAyIDAgb2JqPDwvVHlwZS9QYWdlcy9LaWRzWzMgMCBSXS9Db3VudCAxPj5lbmRvYmogMyAwIG9iajw8L1R5cGUvUGFnZS9NZWRpYUJveFswIDAgMyAzXT4+ZW5kb2JqDQp4cmVmDQowIDQNCjAwMDAwMDAwMDAgNjU1MzUgZg0KMDAwMDAwMDAxMCAwMDAwMCBuDQowMDAwMDAwMDUzIDAwMDAwIG4NCjAwMDAwMDAxMDIgMDAwMDAgbg0KdHJhaWxlcjw8L1NpemUgNC9Sb290IDEgMCBSPj4NCnN0YXJ0eHJlZg0KMTQ5DQolRU9G"
 );
 
-const TEST_DATA_SHORT = "This test string is downloaded.";
-
-/**
- * This is an internal reference that should not be used directly by tests.
- */
-var _gDeferResponses = PromiseUtils.defer();
-
-/**
- * Ensures that all the interruptible requests started after this function is
- * called won't complete until the continueResponses function is called.
- *
- * Normally, the internal HTTP server returns all the available data as soon as
- * a request is received.  In order for some requests to be served one part at a
- * time, special interruptible handlers are registered on the HTTP server.  This
- * allows testing events or actions that need to happen in the middle of a
- * download.
- *
- * For example, the handler accessible at the httpUri("interruptible.txt")
- * address returns the TEST_DATA_SHORT text, then it may block until the
- * continueResponses method is called.  At this point, the handler sends the
- * TEST_DATA_SHORT text again to complete the response.
- *
- * If an interruptible request is started before the function is called, it may
- * or may not be blocked depending on the actual sequence of events.
- */
-function mustInterruptResponses() {
-  // If there are pending blocked requests, allow them to complete.  This is
-  // done to prevent requests from being blocked forever, but should not affect
-  // the test logic, since previously started requests should not be monitored
-  // on the client side anymore.
-  _gDeferResponses.resolve();
-
-  info("Interruptible responses will be blocked midway.");
-  _gDeferResponses = PromiseUtils.defer();
-}
-
-/**
- * Allows all the current and future interruptible requests to complete.
- */
-function continueResponses() {
-  info("Interruptible responses are now allowed to continue.");
-  _gDeferResponses.resolve();
-}
-
-/**
- * Creates a download, which could be interrupted in the middle of it's progress.
- */
-function promiseInterruptibleDownload() {
-  return Downloads.createDownload({
-    source: httpUrl("interruptible.txt"),
-    target: { path: gTestTargetFile.path },
-  });
-}
-
 // 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`);
   }
@@ -158,19 +104,17 @@ function promisePanelOpened() {
 }
 
 async function task_resetState() {
   // Remove all downloads.
   let publicList = await Downloads.getList(Downloads.PUBLIC);
   let downloads = await publicList.getAll();
   for (let download of downloads) {
     publicList.remove(download);
-    if (download?.target.exists) {
-      await download.finalize(true);
-    }
+    await download.finalize(true);
   }
 
   DownloadsPanel.hidePanel();
 
   await promiseFocus();
 }
 
 async function task_addDownloads(aItems) {
@@ -243,31 +187,22 @@ async function setDownloadDir() {
   Services.prefs.setCharPref("browser.download.dir", tmpDir);
   return tmpDir;
 }
 
 let gHttpServer = null;
 function startServer() {
   gHttpServer = new HttpServer();
   gHttpServer.start(-1);
-  registerCleanupFunction(() => {
-    return new Promise(resolve => {
-      // Ensure all the pending HTTP requests have a chance to finish.
-      continueResponses();
-      // Stop the HTTP server, calling resolve when it's done.
+  registerCleanupFunction(async function() {
+    await new Promise(function(resolve) {
       gHttpServer.stop(resolve);
     });
   });
 
-  gHttpServer.identity.setPrimary(
-    "http",
-    "www.example.com",
-    gHttpServer.identity.primaryPort
-  );
-
   gHttpServer.registerPathHandler("/file1.txt", (request, response) => {
     response.setStatusLine(null, 200, "OK");
     response.write("file1");
     response.processAsync();
     response.finish();
   });
   gHttpServer.registerPathHandler("/file2.txt", (request, response) => {
     response.setStatusLine(null, 200, "OK");
@@ -276,42 +211,16 @@ function startServer() {
     response.finish();
   });
   gHttpServer.registerPathHandler("/file3.txt", (request, response) => {
     response.setStatusLine(null, 200, "OK");
     response.write("file3");
     response.processAsync();
     response.finish();
   });
-
-  gHttpServer.registerPathHandler("/interruptible.txt", function(
-    aRequest,
-    aResponse
-  ) {
-    info("Interruptible request started.");
-
-    // Process the first part of the response.
-    aResponse.processAsync();
-    aResponse.setHeader("Content-Type", "text/plain", false);
-    aResponse.setHeader(
-      "Content-Length",
-      "" + TEST_DATA_SHORT.length * 2,
-      false
-    );
-    aResponse.write(TEST_DATA_SHORT);
-
-    // Wait on the current deferred object, then finish the request.
-    _gDeferResponses.promise
-      .then(function RIH_onSuccess() {
-        aResponse.write(TEST_DATA_SHORT);
-        aResponse.finish();
-        info("Interruptible request finished.");
-      })
-      .catch(Cu.reportError);
-  });
 }
 
 function httpUrl(aFileName) {
   return (
     "http://localhost:" + gHttpServer.identity.primaryPort + "/" + aFileName
   );
 }
 
@@ -324,51 +233,16 @@ function openLibrary(aLeftPaneRoot) {
   );
 
   return new Promise(resolve => {
     waitForFocus(resolve, library);
   });
 }
 
 /**
- * Waits for a download to reach its progress, in case it has not
- * reached the expected progress already.
- *
- * @param aDownload
- *        The Download object to wait upon.
- *
- * @return {Promise}
- * @resolves When the download has reached its progress.
- * @rejects Never.
- */
-function promiseDownloadHasProgress(aDownload, progress) {
-  return new Promise(resolve => {
-    // Wait for the download to reach its progress.
-    let onchange = function() {
-      let downloadInProgress =
-        !aDownload.stopped && aDownload.progress == progress;
-      let downloadFinished =
-        progress == 100 &&
-        aDownload.progress == progress &&
-        aDownload.succeeded;
-      if (downloadInProgress || downloadFinished) {
-        info(`Download reached ${progress}%`);
-        aDownload.onchange = null;
-        resolve();
-      }
-    };
-
-    // Register for the notification, but also call the function directly in
-    // case the download already reached the expected progress.
-    aDownload.onchange = onchange;
-    onchange();
-  });
-}
-
-/**
  * Waits for a given button to become visible.
  */
 function promiseButtonShown(id) {
   let dwu = window.windowUtils;
   return BrowserTestUtils.waitForCondition(() => {
     let target = document.getElementById(id);
     let bounds = dwu.getBoundsWithoutFlushing(target);
     return bounds.width > 0 && bounds.height > 0;
--- a/browser/locales/en-US/browser/downloads.ftl
+++ b/browser/locales/en-US/browser/downloads.ftl
@@ -121,32 +121,16 @@ downloads-cmd-choose-open-panel =
 downloads-show-more-information =
     .value = Show more information
 
 # Displayed when hovering a complete download, indicates that it's possible to
 # open the file using an app available in the system.
 downloads-open-file =
     .value = Open File
 
-## Displayed when the user clicked on a download in process. Indicates that the
-## downloading file will be opened after certain amount of time using an app
-## available in the system.
-## Variables:
-##   $hours (number) - Amount of hours left till the file opens.
-##   $seconds (number) - Amount of seconds left till the file opens.
-##   $minutes (number) - Amount of minutes till the file opens.
-
-downloading-file-opens-in-hours-and-minutes = Opening in { $hours }h { $minutes }m…
-downloading-file-opens-in-minutes = Opening in { $minutes }m…
-downloading-file-opens-in-minutes-and-seconds = Opening in { $minutes }m { $seconds }s…
-downloading-file-opens-in-seconds = Opening in { $seconds }s…
-downloading-file-opens-in-some-time = Opening when completed…
-
-##
-
 # Displayed when hovering a download which is able to be retried by users,
 # indicates that it's possible to download this file again.
 downloads-retry-download =
     .value = Retry Download
 
 # Displayed when hovering a download which is able to be cancelled by users,
 # indicates that it's possible to cancel and stop the download.
 downloads-cancel-download =
--- a/browser/themes/shared/downloads/downloads.inc.css
+++ b/browser/themes/shared/downloads/downloads.inc.css
@@ -31,17 +31,16 @@
   padding: 0.31em;
   margin: 0.31em 0;
   border-radius: 4px;
 }
 
 @itemFinished@[exists]:hover:not(.downloadHoveringButton),
 button.downloadButton:hover,
 @item@[verdict]:hover,
-@item@:hover:is(.openWhenFinished),
 .downloadsPanelFooterButton:hover {
   background-color: var(--button-hover-bgcolor);
 }
 
 @itemFinished@[exists]:hover:active,
 button.downloadButton:hover:active,
 .downloadsPanelFooterButton[open="true"],
 @item@[verdict]:hover:active {
--- a/toolkit/mozapps/downloads/DownloadUtils.jsm
+++ b/toolkit/mozapps/downloads/DownloadUtils.jsm
@@ -536,60 +536,16 @@ var DownloadUtils = {
       extra /= timeSize[index];
     }
 
     let value2 = convertTimeUnitsValue(extra);
     let units2 = convertTimeUnitsUnits(value2, nextIndex);
 
     return [value, units, value2, units2];
   },
-
-  /**
-   * Converts a number of seconds to "downloading file opens in X" status.
-   * @param aSeconds
-   *        Seconds to convert into the time format.
-   * @return status object, example:
-   *  status = {
-   *      l10n: {
-   *        id: "downloading-file-opens-in-minutes-and-seconds",
-   *        args: { minutes: 2, seconds: 30 },
-   *      },
-   *   };
-   */
-  getFormattedTimeStatus: function DU_getFormattedTimeStatus(aSeconds) {
-    aSeconds = Math.floor(aSeconds);
-    let l10n;
-    if (!isFinite(aSeconds)) {
-      l10n = {
-        id: "downloading-file-opens-in-some-time",
-      };
-    } else if (aSeconds < 60) {
-      l10n = {
-        id: "downloading-file-opens-in-seconds",
-        args: { seconds: aSeconds },
-      };
-    } else if (aSeconds < 3600) {
-      let minutes = Math.floor(aSeconds / 60);
-      let seconds = aSeconds % 60;
-      l10n = seconds
-        ? {
-            args: { seconds, minutes },
-            id: "downloading-file-opens-in-minutes-and-seconds",
-          }
-        : { args: { minutes }, id: "downloading-file-opens-in-minutes" };
-    } else {
-      let hours = Math.floor(aSeconds / 3600);
-      let minutes = Math.floor((aSeconds % 3600) / 60);
-      l10n = {
-        args: { hours, minutes },
-        id: "downloading-file-opens-in-hours-and-minutes",
-      };
-    }
-    return { l10n };
-  },
 };
 
 /**
  * Private helper for convertTimeUnits that gets the display value of a time
  *
  * @param aTime
  *        Time value for display
  * @return An integer value for the time rounded down