Bug 1452626 - Remove the "download" binding. r=mak,bgrins
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Thu, 04 Oct 2018 11:12:03 +0100
changeset 439828 2614ca0a8884
parent 439827 289fbd180e44
child 439829 54cb6a2f028b
child 439873 2296a2feb192
push id34791
push usertoros@mozilla.com
push dateFri, 05 Oct 2018 21:44:32 +0000
treeherdermozilla-central@54cb6a2f028b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak, bgrins
bugs1452626
milestone64.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 1452626 - Remove the "download" binding. r=mak,bgrins Differential Revision: https://phabricator.services.mozilla.com/D7176
browser/components/downloads/DownloadsSubview.jsm
browser/components/downloads/DownloadsViewUI.jsm
browser/components/downloads/content/allDownloadsView.js
browser/components/downloads/content/download.xml
browser/components/downloads/content/downloads.css
browser/components/downloads/content/downloads.js
--- a/browser/components/downloads/DownloadsSubview.jsm
+++ b/browser/components/downloads/DownloadsSubview.jsm
@@ -416,47 +416,73 @@ DownloadsSubview.Button = class extends 
     if (this._downloadState !== newState) {
       this._downloadState = newState;
       this.onStateChanged();
     } else {
       this._updateState();
     }
   }
 
-  /**
-   * Update the DOM representation of this download to match the current, recently
-   * updated, state.
-   */
+  // DownloadElementShell
+  connect() {}
+
+  // DownloadElementShell
+  showDisplayNameAndIcon(displayName, icon) {
+    this.element.setAttribute("label", displayName);
+    this.element.setAttribute("image", icon);
+  }
+
+  // DownloadElementShell
+  showProgress() {}
+
+  // DownloadElementShell
+  showStatus(status) {
+    this.element.setAttribute("status", status);
+    this.element.setAttribute("tooltiptext", status);
+  }
+
+  // DownloadElementShell
+  showButton() {}
+
+  // DownloadElementShell
+  hideButton() {}
+
+  // DownloadElementShell
   _updateState() {
+    // This view only show completed and failed downloads.
+    let state = DownloadsCommon.stateOfDownload(this.download);
+    let shouldDisplay = state == DownloadsCommon.DOWNLOAD_FINISHED ||
+                        state == DownloadsCommon.DOWNLOAD_FAILED;
+    this.element.hidden = !shouldDisplay;
+    if (!shouldDisplay) {
+      return;
+    }
+
     super._updateState();
-    this.element.setAttribute("label", this.element.getAttribute("displayName"));
-    this.element.setAttribute("tooltiptext", this.element.getAttribute("status"));
 
     if (this.isCommandEnabled("downloadsCmd_show")) {
       this.element.setAttribute("openLabel", kButtonLabels.open);
       this.element.setAttribute("showLabel", kButtonLabels.show);
       this.element.removeAttribute("retryLabel");
     } else if (this.isCommandEnabled("downloadsCmd_retry")) {
       this.element.setAttribute("retryLabel", kButtonLabels.retry);
       this.element.removeAttribute("openLabel");
       this.element.removeAttribute("showLabel");
     } else {
       this.element.removeAttribute("openLabel");
       this.element.removeAttribute("retryLabel");
       this.element.removeAttribute("showLabel");
     }
-
-    this._updateVisibility();
   }
 
-  _updateVisibility() {
-    let state = this.element.getAttribute("state");
-    // This view only show completed and failed downloads.
-    this.element.hidden = !(state == DownloadsCommon.DOWNLOAD_FINISHED ||
-      state == DownloadsCommon.DOWNLOAD_FAILED);
+  // DownloadElementShell
+  _updateStateInner() {
+    if (!this.element.hidden) {
+      super._updateStateInner();
+    }
   }
 
   /**
    * Command handler; copy the download URL to the OS general clipboard.
    */
   downloadsCmd_copyLocation() {
     DownloadsCommon.copyDownloadLink(this.download);
   }
--- a/browser/components/downloads/DownloadsViewUI.jsm
+++ b/browser/components/downloads/DownloadsViewUI.jsm
@@ -62,16 +62,24 @@ var gDownloadElementButtons = {
   },
   removeFile: {
     commandName: "downloadsCmd_confirmBlock",
     l10nId: "download-remove-file",
     iconClass: "downloadIconCancel",
   },
 };
 
+/**
+ * Associates each document with a pre-built DOM fragment representing the
+ * download list item. This is then cloned to create each individual list item.
+ * This is stored on the document to prevent leaks that would occur if a single
+ * instance created by one document's DOMParser was stored globally.
+ */
+var gDownloadListItemFragments = new WeakMap();
+
 var DownloadsViewUI = {
   /**
    * Returns true if the given string is the name of a command that can be
    * handled by the Downloads user interface, including standard commands.
    */
   isCommandName(name) {
     return name.startsWith("cmd_") || name.startsWith("downloadsCmd_");
   },
@@ -136,16 +144,79 @@ this.DownloadsViewUI.DownloadElementShel
 
 this.DownloadsViewUI.DownloadElementShell.prototype = {
   /**
    * The richlistitem for the download, initialized by the derived object.
    */
   element: null,
 
   /**
+   * Manages the "active" state of the shell. By default all the shells are
+   * inactive, thus their UI is not updated. They must be activated when
+   * entering the visible area.
+   */
+  ensureActive() {
+    if (!this._active) {
+      this._active = true;
+      this.connect();
+      this.onChanged();
+    }
+  },
+  get active() {
+    return !!this._active;
+  },
+
+  connect() {
+    let document = this.element.ownerDocument;
+    let downloadListItemFragment = gDownloadListItemFragments.get(document);
+    if (!downloadListItemFragment) {
+      let MozXULElement = document.defaultView.MozXULElement;
+      downloadListItemFragment = MozXULElement.parseXULToFragment(`
+        <hbox class="downloadMainArea" flex="1" align="center">
+          <stack>
+            <image class="downloadTypeIcon" validate="always"/>
+            <image class="downloadBlockedBadge" />
+          </stack>
+          <vbox class="downloadContainer" flex="1" pack="center">
+            <description class="downloadTarget" crop="center"/>
+            <progressmeter class="downloadProgress" min="0" max="100"/>
+            <description class="downloadDetails downloadDetailsNormal"
+                         crop="end"/>
+            <description class="downloadDetails downloadDetailsHover"
+                         crop="end"/>
+            <description class="downloadDetails downloadDetailsButtonHover"
+                         crop="end"/>
+          </vbox>
+        </hbox>
+        <toolbarseparator />
+        <button class="downloadButton"
+                oncommand="DownloadsView.onDownloadButton(event);"/>
+      `);
+      gDownloadListItemFragments.set(document, downloadListItemFragment);
+    }
+    this.element.setAttribute("active", true);
+    this.element.setAttribute("orient", "horizontal");
+    this.element.setAttribute("onclick",
+                              "DownloadsView.onDownloadClick(event);");
+    this.element.appendChild(document.importNode(downloadListItemFragment,
+                                                 true));
+    for (let [propertyName, selector] of [
+      ["_downloadTypeIcon", ".downloadTypeIcon"],
+      ["_downloadTarget", ".downloadTarget"],
+      ["_downloadProgress", ".downloadProgress"],
+      ["_downloadDetailsNormal", ".downloadDetailsNormal"],
+      ["_downloadDetailsHover", ".downloadDetailsHover"],
+      ["_downloadDetailsButtonHover", ".downloadDetailsButtonHover"],
+      ["_downloadButton", ".downloadButton"],
+    ]) {
+      this[propertyName] = this.element.querySelector(selector);
+    }
+  },
+
+  /**
    * Returns a string from the downloads stringbundleset, which contains legacy
    * strings that are loaded from DTD files instead of properties files. This
    * won't be necessary once localization is converted to Fluent (bug 1452637).
    */
   string(l10nId) {
     // These strings are not used often enough to require caching.
     return this.element.ownerDocument.getElementById("downloadsStrings")
                                      .getAttribute("string-" + l10nId);
@@ -170,43 +241,78 @@ this.DownloadsViewUI.DownloadElementShel
            (this.download.succeeded ? "&state=normal" : "");
   },
 
   get browserWindow() {
     return BrowserWindowTracker.getTopWindow();
   },
 
   /**
-   * The progress element for the download, or undefined in case the XBL binding
-   * has not been applied yet.
+   * Updates the display name and icon.
+   *
+   * @param displayName
+   *        This is usually the full file name of the download without the path.
+   * @param icon
+   *        URL of the icon to load, generally from the "image" property.
+   */
+  showDisplayNameAndIcon(displayName, icon) {
+    this._downloadTarget.setAttribute("value", displayName);
+    this._downloadTarget.setAttribute("tooltiptext", displayName);
+    this._downloadTypeIcon.setAttribute("src", icon);
+  },
+
+  /**
+   * Updates the displayed progress bar.
+   *
+   * @param mode
+   *        Either "normal" or "undetermined".
+   * @param value
+   *        Percentage of the progress bar to display, from 0 to 100.
+   * @param paused
+   *        True to display the progress bar style for paused downloads.
    */
-  get _progressElement() {
-    if (!this.__progressElement) {
-      // If the element is not available now, we will try again the next time.
-      this.__progressElement =
-           this.element.ownerDocument.getAnonymousElementByAttribute(
-                                         this.element, "anonid",
-                                         "progressmeter");
+  showProgress(mode, value, paused) {
+    // The "undetermined" mode of the progressmeter is implemented with a
+    // different element structure, and with support from platform code as well.
+    // On Linux only, this mode isn't compatible with the custom styling that we
+    // apply separately with the "progress-undetermined" attribute.
+    this._downloadProgress.setAttribute("mode",
+      AppConstants.platform == "linux" ? "normal" : mode);
+    if (mode == "undetermined") {
+      this._downloadProgress.setAttribute("progress-undetermined", "true");
+    } else {
+      this._downloadProgress.removeAttribute("progress-undetermined");
     }
-    return this.__progressElement;
+    this._downloadProgress.setAttribute("value", value);
+    if (paused) {
+      this._downloadProgress.setAttribute("paused", "true");
+    } else {
+      this._downloadProgress.removeAttribute("paused");
+    }
+
+    // Dispatch the ValueChange event for accessibility.
+    let event = this.element.ownerDocument.createEvent("Events");
+    event.initEvent("ValueChange", true, true);
+    this._downloadProgress.dispatchEvent(event);
   },
 
   /**
    * Updates the full status line.
    *
    * @param status
    *        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.
    */
   showStatus(status, hoverStatus = status) {
-    this.element.setAttribute("status", status);
-    this.element.setAttribute("hoverStatus", hoverStatus);
+    this._downloadDetailsNormal.setAttribute("value", status);
+    this._downloadDetailsNormal.setAttribute("tooltiptext", status);
+    this._downloadDetailsHover.setAttribute("value", hoverStatus);
   },
 
   /**
    * Updates the status line combining the given state label with other labels.
    *
    * @param stateLabel
    *        Label representing the state of the download, for example "Failed".
    *        In the Downloads Panel, this is the only text displayed when the
@@ -233,45 +339,50 @@ this.DownloadsViewUI.DownloadElementShel
 
     if (!this.isPanel) {
       this.showStatus(fullStatus);
     } else {
       this.showStatus(stateLabel, hoverStatus || fullStatus);
     }
   },
 
+  /**
+   * Updates the main action button and makes it visible.
+   *
+   * @param type
+   *        One of the presets defined in gDownloadElementButtons.
+   */
   showButton(type) {
     let { commandName, l10nId, descriptionL10nId,
           iconClass } = gDownloadElementButtons[type];
 
     this.buttonCommandName = commandName;
-    let labelAttribute = this.isPanel ? "buttonarialabel" : "buttontooltiptext";
-    this.element.setAttribute(labelAttribute, this.string(l10nId));
+    let labelAttribute = this.isPanel ? "aria-label" : "tooltiptext";
+    this._downloadButton.setAttribute(labelAttribute, this.string(l10nId));
     if (this.isPanel && descriptionL10nId) {
-      this.element.setAttribute("buttonHoverStatus",
-                                this.string(descriptionL10nId));
+      this._downloadDetailsButtonHover.setAttribute("value",
+        this.string(descriptionL10nId));
     }
-    this.element.setAttribute("buttonclass", "downloadButton " + iconClass);
-    this.element.removeAttribute("buttonhidden");
+    this._downloadButton.setAttribute("class", "downloadButton " + iconClass);
+    this._downloadButton.removeAttribute("hidden");
   },
 
   hideButton() {
-    this.element.setAttribute("buttonhidden", "true");
+    this._downloadButton.setAttribute("hidden", "true");
   },
 
   lastEstimatedSecondsLeft: Infinity,
 
   /**
    * This is called when a major state change occurs in the download, but is not
    * called for every progress update in order to improve performance.
    */
   _updateState() {
-    this.element.setAttribute("displayName",
-                              DownloadsViewUI.getDisplayName(this.download));
-    this.element.setAttribute("image", this.image);
+    this.showDisplayNameAndIcon(DownloadsViewUI.getDisplayName(this.download),
+                                this.image);
     this.element.setAttribute("state",
                               DownloadsCommon.stateOfDownload(this.download));
 
     if (!this.download.stopped) {
       // When the download becomes in progress, we make all the major changes to
       // the user interface here. The _updateStateInner function takes care of
       // displaying the right button type for all other state changes.
       this.showButton("cancel");
@@ -419,43 +530,21 @@ this.DownloadsViewUI.DownloadElementShel
         this.element.removeAttribute("verdict");
       }
 
       this.element.classList.toggle("temporary-block",
                                     !!this.download.hasBlockedData);
     }
 
     // These attributes are set in all code paths, because they are relevant for
-    // downloads that are in progress and for other states where the progress
-    // bar is visible.
+    // downloads that are in progress and for other states.
     if (this.download.hasProgress) {
-      this.element.setAttribute("progressmode", "normal");
-      this.element.setAttribute("progress", this.download.progress);
-      this.element.removeAttribute("progress-undetermined");
+      this.showProgress("normal", this.download.progress, progressPaused);
     } else {
-      // Suppress the progress animation on Linux for the Downloads Panel
-      // progress bars when the file size is unknown.
-      this.element.setAttribute("progressmode",
-                                AppConstants.platform == "linux" ? "normal" :
-                                                                   "undetermined");
-      this.element.setAttribute("progress-undetermined", "true");
-      this.element.setAttribute("progress", "100");
-    }
-
-    if (progressPaused) {
-      this.element.setAttribute("progresspaused", "true");
-    } else {
-      this.element.removeAttribute("progresspaused");
-    }
-
-    // Dispatch the ValueChange event for accessibility, if possible.
-    if (this._progressElement) {
-      let event = this.element.ownerDocument.createEvent("Events");
-      event.initEvent("ValueChange", true, true);
-      this._progressElement.dispatchEvent(event);
+      this.showProgress("undetermined", 100, progressPaused);
     }
   },
 
   /**
    * Returns [title, [details1, details2]] for blocked downloads.
    */
   get rawBlockedTitleAndDetails() {
     let s = DownloadsCommon.strings;
--- a/browser/components/downloads/content/allDownloadsView.js
+++ b/browser/components/downloads/content/allDownloadsView.js
@@ -44,32 +44,16 @@ function HistoryDownloadElementShell(dow
   this.element.classList.add("download");
   this.element.classList.add("download-state");
 }
 
 HistoryDownloadElementShell.prototype = {
   __proto__: DownloadsViewUI.DownloadElementShell.prototype,
 
   /**
-   * Manages the "active" state of the shell.  By default all the shells are
-   * inactive, thus their UI is not updated.  They must be activated when
-   * entering the visible area.
-   */
-  ensureActive() {
-    if (!this._active) {
-      this._active = true;
-      this.element.setAttribute("active", true);
-      this.onChanged();
-    }
-  },
-  get active() {
-    return !!this._active;
-  },
-
-  /**
    * Overrides the base getter to return the Download or HistoryDownload object
    * for displaying information and executing commands in the user interface.
    */
   get download() {
     return this._download;
   },
 
   onStateChanged() {
--- a/browser/components/downloads/content/download.xml
+++ b/browser/components/downloads/content/download.xml
@@ -7,72 +7,22 @@
    - You can obtain one at http://mozilla.org/MPL/2.0/. -->
 
 <!DOCTYPE bindings SYSTEM "chrome://browser/locale/downloads/downloads.dtd">
 
 <bindings id="downloadBindings"
           xmlns="http://www.mozilla.org/xbl"
           xmlns:xul="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
           xmlns:xbl="http://www.mozilla.org/xbl">
-
-  <binding id="download"
-           extends="chrome://global/content/bindings/richlistbox.xml#richlistitem">
-    <content orient="horizontal"
-             onclick="DownloadsView.onDownloadClick(event);">
-      <xul:hbox class="downloadMainArea"
-                flex="1"
-                align="center">
-        <xul:stack>
-          <xul:image class="downloadTypeIcon"
-                     validate="always"
-                     xbl:inherits="src=image"/>
-          <xul:image class="downloadBlockedBadge" />
-        </xul:stack>
-        <xul:vbox pack="center"
-                  flex="1"
-                  class="downloadContainer">
-          <!-- We're letting localizers put a min-width in here primarily
-               because of the downloads summary at the bottom of the list of
-               download items. An element in the summary has the same min-width
-               on a description, and we don't want the panel to change size if the
-               summary isn't being displayed, so we ensure that items share the
-               same minimum width.
-               -->
-          <xul:description class="downloadTarget"
-                           crop="center"
-                           xbl:inherits="value=displayName,tooltiptext=displayName"/>
-          <xul:progressmeter anonid="progressmeter"
-                             class="downloadProgress"
-                             min="0"
-                             max="100"
-                             xbl:inherits="progress-undetermined,mode=progressmode,value=progress,paused=progresspaused"/>
-          <xul:description class="downloadDetails downloadDetailsNormal"
-                           crop="end"
-                           xbl:inherits="value=status,tooltiptext=status"/>
-          <xul:description class="downloadDetails downloadDetailsHover"
-                           crop="end"
-                           xbl:inherits="value=hoverStatus"/>
-          <xul:description class="downloadDetails downloadDetailsButtonHover"
-                           crop="end"
-                           xbl:inherits="value=buttonHoverStatus"/>
-        </xul:vbox>
-      </xul:hbox>
-      <xul:toolbarseparator />
-      <xul:button class="downloadButton"
-                  xbl:inherits="class=buttonclass,aria-label=buttonarialabel,tooltiptext=buttontooltiptext"
-                  oncommand="DownloadsView.onDownloadButton(event);"/>
-    </content>
-  </binding>
-
   <binding id="download-subview-toolbarbutton"
            extends="chrome://global/content/bindings/button.xml#button-base">
     <content>
-      <xul:image class="toolbarbutton-icon" xbl:inherits="validate,src=image,label,consumeanchor"/>
+      <xul:image class="toolbarbutton-icon" validate="always" xbl:inherits="src=image"/>
       <xul:vbox class="toolbarbutton-text" flex="1">
-        <xul:label crop="end" xbl:inherits="value=label,accesskey,crop,wrap"/>
+        <xul:label crop="end" xbl:inherits="value=label"/>
         <xul:label class="status-text status-full" crop="end" xbl:inherits="value=status"/>
         <xul:label class="status-text status-open" crop="end" xbl:inherits="value=openLabel"/>
         <xul:label class="status-text status-retry" crop="end" xbl:inherits="value=retryLabel"/>
         <xul:label class="status-text status-show" crop="end" xbl:inherits="value=showLabel"/>
       </xul:vbox>
       <xul:toolbarbutton anonid="button" class="action-button"/>
     </content>
   </binding>
--- a/browser/components/downloads/content/downloads.css
+++ b/browser/components/downloads/content/downloads.css
@@ -1,18 +1,14 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /*** Downloads Panel ***/
 
-#downloadsListBox > richlistitem {
-  -moz-binding: url('chrome://browser/content/downloads/download.xml#download');
-}
-
 #downloadsListBox > richlistitem:not([selected]) button {
   /* Only focus buttons in the selected item. */
   -moz-user-focus: none;
 }
 
 #downloadsSummary:not([inprogress]) > vbox > #downloadsSummaryProgress,
 #downloadsSummary:not([inprogress]) > vbox > #downloadsSummaryDetails,
 #downloadsFooter:not([showingsummary]) #downloadsSummary {
@@ -39,24 +35,20 @@
  * This hack makes sure we don't apply any binding to inactive items (inactive
  * items are history downloads that haven't been in the visible area).
  * We can do this because the richlistbox implementation does not interact
  * much with the richlistitem binding.  However, this may turn out to have
  * some side effects (see bug 828111 for the details).
  *
  * We might be able to do away with this workaround once bug 653881 is fixed.
  */
-#downloadsRichListBox > richlistitem {
+#downloadsRichListBox > richlistitem:not([active]) {
   -moz-binding: none;
 }
 
-#downloadsRichListBox > richlistitem[active] {
-  -moz-binding: url("chrome://browser/content/downloads/download.xml#download");
-}
-
 #downloadsRichListBox > richlistitem button {
   /* These buttons should never get focus, as that would "disable"
      the downloads view controller (it's only used when the richlistbox
      is focused). */
   -moz-user-focus: none;
 }
 
 /*** Visibility of controls inside download items ***/
--- a/browser/components/downloads/content/downloads.js
+++ b/browser/components/downloads/content/downloads.js
@@ -727,16 +727,17 @@ var DownloadsView = {
     let viewItem = new DownloadsViewItem(download, element);
     this._visibleViewItems.set(download, viewItem);
     this._itemsForElements.set(element, viewItem);
     if (aNewest) {
       this.richListBox.insertBefore(element, this.richListBox.firstElementChild);
     } else {
       this.richListBox.appendChild(element);
     }
+    viewItem.ensureActive();
   },
 
   /**
    * Removes the view item associated with the specified data item.
    */
   _removeViewItem(download) {
     DownloadsCommon.log("Removing a DownloadsViewItem from the downloads list.");
     let element = this._visibleViewItems.get(download).element;
@@ -906,39 +907,36 @@ XPCOMUtils.defineConstant(this, "Downloa
  *
  * @param download
  *        Download object to be associated with the view item.
  * @param aElement
  *        XUL element corresponding to the single download item in the view.
  */
 function DownloadsViewItem(download, aElement) {
   this.download = download;
-  this.downloadState = DownloadsCommon.stateOfDownload(download);
   this.element = aElement;
   this.element._shell = this;
 
   this.element.setAttribute("type", "download");
   this.element.classList.add("download-state");
 
   this.isPanel = true;
-
-  this._updateState();
 }
 
 DownloadsViewItem.prototype = {
   __proto__: DownloadsViewUI.DownloadElementShell.prototype,
 
   /**
    * The XUL element corresponding to the associated richlistbox item.
    */
   _element: null,
 
   onChanged() {
     let newState = DownloadsCommon.stateOfDownload(this.download);
-    if (this.downloadState != newState) {
+    if (this.downloadState !== newState) {
       this.downloadState = newState;
       this._updateState();
     } else {
       this._updateStateInner();
     }
   },
 
   isCommandEnabled(aCommand) {