Bug 1354532 - Part 2 - Generalize the Downloads commands handling across panel and library window. r?paolo draft
authorMike de Boer <mdeboer@mozilla.com>
Tue, 11 Jul 2017 12:23:32 +0200
changeset 606720 b236c72b51e738c02073cbe6c79cd99564077eeb
parent 606719 beb010a98fdbd0bdb8c14cb0c19f856ed253b7a8
child 606721 ec27c5015577faf4b82a170cfd0df1885f2f14b5
push id67791
push usermdeboer@mozilla.com
push dateTue, 11 Jul 2017 10:33:03 +0000
reviewerspaolo
bugs1354532
milestone56.0a1
Bug 1354532 - Part 2 - Generalize the Downloads commands handling across panel and library window. r?paolo Mixed and moved the commands to a separate include file, which is used in browser-sets.inc and allDownloadsViewOverlay.xul. The net result of this unification is that command handling is now more similar to how it's done and generalized in the Places views. MozReview-Commit-ID: HITAOnFvci6
browser/base/content/browser-sets.inc
browser/components/downloads/DownloadsViewUI.jsm
browser/components/downloads/content/allDownloadsViewOverlay.js
browser/components/downloads/content/allDownloadsViewOverlay.xul
browser/components/downloads/content/commands.inc.xul
browser/components/downloads/content/downloads.js
browser/components/downloads/content/downloadsOverlay.xul
--- a/browser/base/content/browser-sets.inc
+++ b/browser/base/content/browser-sets.inc
@@ -119,16 +119,20 @@
 
   <commandset id="placesCommands">
     <command id="Browser:ShowAllBookmarks"
              oncommand="PlacesCommandHook.showPlacesOrganizer('UnfiledBookmarks');"/>
     <command id="Browser:ShowAllHistory"
              oncommand="PlacesCommandHook.showPlacesOrganizer('History');"/>
   </commandset>
 
+  <commandset id="downloadCommands">
+#include ../../components/downloads/content/commands.inc.xul
+  </commandset>
+
   <broadcasterset id="mainBroadcasterSet">
     <broadcaster id="Social:PageShareable" disabled="true"/>
     <broadcaster id="viewBookmarksSidebar" autoCheck="false" label="&bookmarksButton.label;"
                  type="checkbox" group="sidebar" sidebarurl="chrome://browser/content/bookmarks/bookmarksPanel.xul"
                  oncommand="SidebarUI.toggle('viewBookmarksSidebar');"/>
 
     <!-- for both places and non-places, the sidebar lives at
          chrome://browser/content/history/history-panel.xul so there are no
--- a/browser/components/downloads/DownloadsViewUI.jsm
+++ b/browser/components/downloads/DownloadsViewUI.jsm
@@ -944,17 +944,17 @@ this.DownloadsViewUI.HistoryDownloadElem
   if (aSessionDownload) {
     this.sessionDownload = aSessionDownload;
   }
   if (aHistoryDownload) {
     this.historyDownload = aHistoryDownload;
   }
 
   this.window = aWindow;
-  this.controller = new DownloadsViewUI.DownloadsPlacesViewController(this, aWindow);
+  this.controller = new DownloadsViewUI.DownloadsPlacesViewController(this);
 }
 
 this.DownloadsViewUI.HistoryDownloadElementShell.prototype = {
   __proto__: this.DownloadsViewUI.DownloadElementShell.prototype,
 
   /**
    * Manages the "active" state of the shell.  By default all the shells without
    * a session download are inactive, thus their UI is not updated.  They must
@@ -1182,23 +1182,76 @@ this.DownloadsViewUI.HistoryDownloadElem
     }
 
     // Ensure the interface has been updated based on the new values. We need to
     // do this because history downloads can't trigger update notifications.
     this._updateProgress();
   },
 };
 
+let gSupportedCommands = new WeakMap();
+
 this.DownloadsViewUI.DownloadsPlacesViewController = class {
-  constructor(view, global) {
+  constructor(view) {
     this.view = view;
-    this.global = global;
+  }
+
+  terminate() {
+    this.unregister();
+    this.view = null;
+  }
+
+  register() {
+    if (!this.view)
+      return;
+
+    DownloadsViewUI.DownloadsPlacesViewController.setSupportedCommands(this.view);
+    this.view.window.controllers.appendController(this);
+  }
+
+  unregister() {
+    if (!this.view)
+      return;
+
+    DownloadsViewUI.DownloadsPlacesViewController.unsetSupportedCommands(this.view);
+    this.view.window.controllers.removeController(this);
   }
 
-  terminate() {}
+  static setSupportedCommands(view) {
+    let window = view.window;
+    let commands = gSupportedCommands.get(window);
+    if (!commands)
+      gSupportedCommands.set(window, commands = {});
+
+    let names = [...Object.getOwnPropertyNames(view), ...Object.getOwnPropertyNames(view.__proto__)];
+    for (let name of names) {
+      if (!DownloadsViewUI.isCommandName(name))
+        continue;
+
+      if (!commands[name])
+        commands[name] = 0;
+      ++commands[name];
+    }
+  }
+
+  static unsetSupportedCommands(view) {
+    let window = view.window;
+    let commands = gSupportedCommands.get(window);
+    if (!commands)
+      gSupportedCommands.set(window, commands = {});
+
+    for (let name in commands) {
+      if (!--commands[name])
+        delete commands[name];
+    }
+  }
+
+  static getSupportedCommands(window) {
+    return gSupportedCommands.get(window) || {};
+  }
 
   supportsCommand(aCommand) {
     // Firstly, determine if this is a command that we can handle.
     if (!DownloadsViewUI.isCommandName(aCommand)) {
       return false;
     }
     if (!(aCommand in this) &&
         !(aCommand in DownloadsViewUI.HistoryDownloadElementShell.prototype)) {
@@ -1206,17 +1259,17 @@ this.DownloadsViewUI.DownloadsPlacesView
     }
     // If this function returns true, other controllers won't get a chance to
     // process the command even if isCommandEnabled returns false, so it's
     // important to check if the list is focused here to handle common commands
     // like copy and paste correctly. The clear downloads command, instead, is
     // specific to the downloads list but can be invoked from the toolbar, so we
     // can just return true unconditionally.
     return aCommand == "downloadsCmd_clearDownloads" ||
-           this.global.document.activeElement == this.view._richlistbox;
+           this.view.window.document.activeElement == this.view._richlistbox;
   }
 
   isCommandEnabled(aCommand) {
     switch (aCommand) {
       case "cmd_copy":
       case "downloadsCmd_openReferrer":
       case "downloadShowMenuItem":
         return this.view._richlistbox.selectedItems.length == 1;
@@ -1281,19 +1334,18 @@ this.DownloadsViewUI.DownloadsPlacesView
 
   _canDownloadClipboardURL() {
     let [url /* ,name */] = this._getURLFromClipboardData();
     return url != "";
   }
 
   _downloadURLFromClipboard() {
     let [url, name] = this._getURLFromClipboardData();
-    let browserWin = RecentWindow.getMostRecentBrowserWindow();
-    let initiatingDoc = browserWin ? browserWin.document : this.global.document;
-    this.global.DownloadURL(url, name, initiatingDoc);
+    let window = this.view.window;
+    window.DownloadURL(url, name, window.document);
   }
 
   doCommand(aCommand) {
     // Commands may be invoked with keyboard shortcuts even if disabled.
     if (!this.isCommandEnabled(aCommand)) {
       return;
     }
 
@@ -1332,30 +1384,30 @@ this.DownloadsViewUI.DownloadsPlacesView
     this.view._downloadsData.removeFinished();
     if (this.view.result) {
       Cc["@mozilla.org/browser/download-history;1"]
         .getService(Ci.nsIDownloadHistory)
         .removeAllDownloads();
     }
     // There may be no selection or focus change as a result
     // of these change, and we want the command updated immediately.
-    this.global.goUpdateCommand("downloadsCmd_clearDownloads");
+    this.view.window.goUpdateCommand("downloadsCmd_clearDownloads");
   }
 
   buildContextMenu(contextMenu, download) {
     contextMenu.setAttribute("state",
                              DownloadsCommon.stateOfDownload(download));
     contextMenu.setAttribute("exists", "true");
     contextMenu.classList.toggle("temporary-block",
                                  !!download.hasBlockedData);
 
     if (!download.stopped) {
       // The hasPartialData property of a download may change at any time after
       // it has started, so ensure we update the related command now.
-      this.global.goUpdateCommand("downloadsCmd_pauseResume");
+      this.view.window.goUpdateCommand("downloadsCmd_pauseResume");
     }
   }
 
   setDataTransfer(aEvent) {
     // TODO Bug 831358: Support d&d for multiple selection.
     // For now, we just drag the first element.
     let selectedItem = this.view._richlistbox.selectedItem;
     if (!selectedItem) {
--- a/browser/components/downloads/content/allDownloadsViewOverlay.js
+++ b/browser/components/downloads/content/allDownloadsViewOverlay.js
@@ -13,49 +13,49 @@ XPCOMUtils.defineLazyModuleGetter(this, 
                                   "resource://gre/modules/osfile.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils",
                                   "resource://gre/modules/PlacesUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "RecentWindow",
                                   "resource:///modules/RecentWindow.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Services",
                                   "resource://gre/modules/Services.jsm");
 
-var gController = null;
-
 /**
  * Relays commands from the download.xml binding to the selected items.
  */
 const DownloadsView = window.DownloadsView = {
   onDownloadCommand(event, command) {
     goDoCommand(command);
   },
 
   getControllerForCommand(command) {
-    return gController;
+    return window.controllers.getControllerForCommand(command);
   },
 
   goUpdateCommands() {
-    if (!gController)
+    let controller = this.getControllerForCommand("downloadsCmd_open");
+    if (!controller)
       return;
 
     let updateCommandsForObject = object => {
       for (let name in object) {
         if (DownloadsViewUI.isCommandName(name)) {
-          goSetCommandEnabled(name, gController.isCommandEnabled(name));
+          goSetCommandEnabled(name, controller.isCommandEnabled(name));
         }
       }
     };
 
-    updateCommandsForObject(gController.view);
+    updateCommandsForObject(DownloadsViewUI.DownloadsPlacesViewController.getSupportedCommands(window));
     updateCommandsForObject(DownloadsViewUI.HistoryDownloadElementShell.prototype);
   },
 
   goDoCommand(command) {
-    if (gController && gController.isCommandEnabled(command))
-      gController.doCommand(command);
+    let controller = this.getControllerForCommand("downloadsCmd_open");
+    if (controller.isCommandEnabled(command))
+      controller.doCommand(command);
   },
 
   onDownloadClick() {},
 };
 
 /**
  * A Downloads Places View is a places view designed to show a places query
  * for history downloads alongside the session downloads.
@@ -67,28 +67,27 @@ const DownloadsView = window.DownloadsVi
  * download, or both. Session downloads are shown first in the view, and as long
  * as they exist they "collapses" their history "counterpart" (So we don't show two
  * items for every download).
  */
 function DownloadsPlacesView(aRichListBox, aActive = true) {
   this._richlistbox = aRichListBox;
   this._richlistbox._placesView = this;
 
-  gController = new DownloadsViewUI.DownloadsPlacesViewController(this, window);
-  window.controllers.insertControllerAt(0, gController);
-
   this.init(window, aActive);
 
+  this.controller = new DownloadsViewUI.DownloadsPlacesViewController(this);
+  this.controller.register();
   this._initiallySelectedElement = null;
 
   // Make sure to unregister the view if the window is closed.
   window.addEventListener("unload", () => {
-    window.controllers.removeController(gController);
+    this.controller.terminate();
     this.destructor();
-    this.result = gController = null;
+    this.result = this.controller = null;
   }, true);
   // Resizing the window may change items visibility.
   window.addEventListener("resize", () => {
     this._ensureVisibleElementsAreActive();
   }, true);
 }
 
 DownloadsPlacesView.prototype = {
@@ -335,20 +334,16 @@ DownloadsPlacesView.prototype = {
   nodeLastModifiedChanged() {},
   nodeHistoryDetailsChanged() {},
   nodeTagsChanged() {},
   sortingChanged() {},
   nodeMoved() {},
   nodeURIChanged() {},
   batching() {},
 
-  get controller() {
-    return this._richlistbox.controller;
-  },
-
   /**
    * When the view loads, we want to select the first item.
    * However, because session downloads, for which the data is loaded
    * asynchronously, always come first in the list, and because the list
    * may (or may not) already contain history downloads at that point, it
    * turns out that by the time we can select the first item, the user may
    * have already started using the view.
    * To make things even more complicated, in other cases, the places data
@@ -382,17 +377,17 @@ DownloadsPlacesView.prototype = {
     let element = this._richlistbox.selectedItem;
     if (!element || !element._shell) {
       return false;
     }
 
     // Set the state attribute so that only the appropriate items are displayed.
     let contextMenu = document.getElementById("downloadsContextMenu");
     let download = element._shell.download;
-    gController.buildContextMenu(contextMenu, download);
+    this.controller.buildContextMenu(contextMenu, download);
 
     return true;
   },
 
   onKeyPress(aEvent) {
     let selectedElements = this._richlistbox.selectedItems;
     if (aEvent.keyCode == KeyEvent.DOM_VK_RETURN) {
       // In the content tree, opening bookmarks by pressing return is only
@@ -441,17 +436,17 @@ DownloadsPlacesView.prototype = {
     for (let elt of selectedElements) {
       if (elt._shell) {
         elt._shell.onSelect();
       }
     }
   },
 
   onDragStart(aEvent) {
-    gController.setDataTransfer(aEvent);
+    this.controller.setDataTransfer(aEvent);
   },
 
   onDragOver(aEvent) {
     let types = aEvent.dataTransfer.types;
     if (types.includes("text/uri-list") ||
         types.includes("text/x-moz-url") ||
         types.includes("text/plain")) {
       aEvent.preventDefault();
--- a/browser/components/downloads/content/allDownloadsViewOverlay.xul
+++ b/browser/components/downloads/content/allDownloadsViewOverlay.xul
@@ -52,38 +52,17 @@
                onfocus="DownloadsView.goUpdateCommands();"
                onselect="this._placesView.onSelect();"
                onblur="DownloadsView.goUpdateCommands();"/>
 
   <commandset id="downloadCommands"
               commandupdater="true"
               events="focus,select,contextmenu"
               oncommandupdate="DownloadsView.goUpdateCommands();">
-    <command id="downloadsCmd_pauseResume"
-             oncommand="DownloadsView.goDoCommand('downloadsCmd_pauseResume')"/>
-    <command id="downloadsCmd_cancel"
-             oncommand="DownloadsView.goDoCommand('downloadsCmd_cancel')"/>
-    <command id="downloadsCmd_unblock"
-             oncommand="DownloadsView.goDoCommand('downloadsCmd_unblock')"/>
-    <command id="downloadsCmd_chooseUnblock"
-             oncommand="DownloadsView.goDoCommand('downloadsCmd_chooseUnblock')"/>
-    <command id="downloadsCmd_chooseOpen"
-             oncommand="DownloadsView.goDoCommand('downloadsCmd_chooseOpen')"/>
-    <command id="downloadsCmd_confirmBlock"
-             oncommand="DownloadsView.goDoCommand('downloadsCmd_confirmBlock')"/>
-    <command id="downloadsCmd_open"
-             oncommand="DownloadsView.goDoCommand('downloadsCmd_open')"/>
-    <command id="downloadsCmd_show"
-             oncommand="DownloadsView.goDoCommand('downloadsCmd_show')"/>
-    <command id="downloadsCmd_retry"
-             oncommand="DownloadsView.goDoCommand('downloadsCmd_retry')"/>
-    <command id="downloadsCmd_openReferrer"
-             oncommand="DownloadsView.goDoCommand('downloadsCmd_openReferrer')"/>
-    <command id="downloadsCmd_clearDownloads"
-             oncommand="DownloadsView.goDoCommand('downloadsCmd_clearDownloads')"/>
+#include commands.inc.xul
   </commandset>
 
   <menupopup id="downloadsContextMenu" class="download-state">
     <menuitem command="downloadsCmd_pauseResume"
               class="downloadPauseMenuItem"
               label="&cmd.pause.label;"
               accesskey="&cmd.pause.accesskey;"/>
     <menuitem command="downloadsCmd_pauseResume"
new file mode 100644
--- /dev/null
+++ b/browser/components/downloads/content/commands.inc.xul
@@ -0,0 +1,30 @@
+<command id="downloadsCmd_doDefault"
+         oncommand="DownloadsView.goDoCommand('downloadsCmd_doDefault')"/>
+<command id="downloadsCmd_pauseResume"
+         oncommand="DownloadsView.goDoCommand('downloadsCmd_pauseResume')"/>
+<command id="downloadsCmd_cancel"
+         oncommand="DownloadsView.goDoCommand('downloadsCmd_cancel')"/>
+<command id="downloadsCmd_unblock"
+         oncommand="DownloadsView.goDoCommand('downloadsCmd_unblock')"/>
+<command id="downloadsCmd_chooseUnblock"
+         oncommand="DownloadsView.goDoCommand('downloadsCmd_chooseUnblock')"/>
+<command id="downloadsCmd_chooseOpen"
+         oncommand="DownloadsView.goDoCommand('downloadsCmd_chooseOpen')"/>
+<command id="downloadsCmd_unblockAndOpen"
+         oncommand="DownloadsView.goDoCommand('downloadsCmd_unblockAndOpen')"/>
+<command id="downloadsCmd_confirmBlock"
+         oncommand="DownloadsView.goDoCommand('downloadsCmd_confirmBlock')"/>
+<command id="downloadsCmd_open"
+         oncommand="DownloadsView.goDoCommand('downloadsCmd_open')"/>
+<command id="downloadsCmd_show"
+         oncommand="DownloadsView.goDoCommand('downloadsCmd_show')"/>
+<command id="downloadsCmd_retry"
+         oncommand="DownloadsView.goDoCommand('downloadsCmd_retry')"/>
+<command id="downloadsCmd_openReferrer"
+         oncommand="DownloadsView.goDoCommand('downloadsCmd_openReferrer')"/>
+<command id="downloadsCmd_copyLocation"
+         oncommand="DownloadsView.goDoCommand('downloadsCmd_copyLocation')"/>
+<command id="downloadsCmd_clearList"
+         oncommand="DownloadsView.goDoCommand('downloadsCmd_clearList')"/>
+<command id="downloadsCmd_clearDownloads"
+         oncommand="DownloadsView.goDoCommand('downloadsCmd_clearDownloads')"/>
--- a/browser/components/downloads/content/downloads.js
+++ b/browser/components/downloads/content/downloads.js
@@ -1046,16 +1046,67 @@ const DownloadsView = {
     dataTransfer.effectAllowed = "copyMove";
     let spec = NetUtil.newURI(file).spec;
     dataTransfer.setData("text/uri-list", spec);
     dataTransfer.setData("text/plain", spec);
     dataTransfer.addElement(element);
 
     aEvent.stopPropagation();
   },
+
+  getControllerForCommand(command) {
+    // A context menu may be built for non-focusable views.  Thus, we first try
+    // to look for a view associated with document.popupNode
+    let popupNode;
+    try {
+      popupNode = document.popupNode;
+    } catch (e) {
+      // The document went away (bug 797307).
+      return null;
+    }
+    if (popupNode) {
+      let view = PlacesUIUtils.getViewForNode(popupNode);
+      if (view && view._contextMenuShown) {
+        return view.controllers.getControllerForCommand(command);
+      }
+    }
+
+    // When we're not building a context menu, only focusable views
+    // are possible.  Thus, we can safely use the command dispatcher.
+    let controller = top.document.commandDispatcher
+                        .getControllerForCommand(command);
+    if (controller)
+      return controller;
+
+    return null;
+  },
+
+  goUpdateCommands() {
+    // Get the controller for one of the download commands.
+    let controller = this.getControllerForCommand("downloadsCmd_open");
+    if (!controller)
+      return;
+
+    let updateCommandsForObject = object => {
+      for (let name in object) {
+        if (DownloadsViewUI.isCommandName(name)) {
+          goSetCommandEnabled(name, controller.isCommandEnabled(name));
+        }
+      }
+    };
+
+    updateCommandsForObject(DownloadsViewUI.DownloadsPlacesViewController.getSupportedCommands(window));
+    updateCommandsForObject(DownloadsViewUI.HistoryDownloadElementShell.prototype);
+  },
+
+  goDoCommand(command) {
+    let controller = this.getControllerForCommand(command);
+    if (controller && controller.isCommandEnabled(command))
+      controller.doCommand(command);
+  },
 }
 
 XPCOMUtils.defineConstant(this, "DownloadsView", DownloadsView);
 
 // DownloadsViewItem
 
 /**
  * Builds and updates a single item in the downloads list widget, responding to
--- a/browser/components/downloads/content/downloadsOverlay.xul
+++ b/browser/components/downloads/content/downloadsOverlay.xul
@@ -9,58 +9,30 @@
 <?xml-stylesheet href="chrome://browser/skin/downloads/downloads.css"?>
 
 <!DOCTYPE overlay SYSTEM "chrome://browser/locale/downloads/downloads.dtd">
 
 <overlay xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
          xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
          id="downloadsOverlay">
 
-  <commandset>
-    <command id="downloadsCmd_doDefault"
-             oncommand="goDoCommand('downloadsCmd_doDefault')"/>
-    <command id="downloadsCmd_pauseResume"
-             oncommand="goDoCommand('downloadsCmd_pauseResume')"/>
-    <command id="downloadsCmd_cancel"
-             oncommand="goDoCommand('downloadsCmd_cancel')"/>
-    <command id="downloadsCmd_unblock"
-             oncommand="goDoCommand('downloadsCmd_unblock')"/>
-    <command id="downloadsCmd_chooseUnblock"
-             oncommand="goDoCommand('downloadsCmd_chooseUnblock')"/>
-    <command id="downloadsCmd_unblockAndOpen"
-             oncommand="goDoCommand('downloadsCmd_unblockAndOpen')"/>
-    <command id="downloadsCmd_confirmBlock"
-             oncommand="goDoCommand('downloadsCmd_confirmBlock')"/>
-    <command id="downloadsCmd_open"
-             oncommand="goDoCommand('downloadsCmd_open')"/>
-    <command id="downloadsCmd_show"
-             oncommand="goDoCommand('downloadsCmd_show')"/>
-    <command id="downloadsCmd_retry"
-             oncommand="goDoCommand('downloadsCmd_retry')"/>
-    <command id="downloadsCmd_openReferrer"
-             oncommand="goDoCommand('downloadsCmd_openReferrer')"/>
-    <command id="downloadsCmd_copyLocation"
-             oncommand="goDoCommand('downloadsCmd_copyLocation')"/>
-    <command id="downloadsCmd_clearList"
-             oncommand="goDoCommand('downloadsCmd_clearList')"/>
-  </commandset>
-
   <popupset id="mainPopupSet">
     <!-- The panel has level="top" to ensure that it is never hidden by the
          taskbar on Windows.  See bug 672365.  For accessibility to screen
          readers, we use a label on the panel instead of the anchor because the
          panel can also be displayed without an anchor. -->
     <panel id="downloadsPanel"
            aria-label="&downloads.title;"
            role="group"
            type="arrow"
            orient="vertical"
            level="top"
            onpopupshown="DownloadsPanel.onPopupShown(event);"
            onpopuphidden="DownloadsPanel.onPopupHidden(event);">
+      <commandset id="downloadCommands"/>
       <!-- The following popup menu should be a child of the panel element,
            otherwise flickering may occur when the cursor is moved over the area
            of a disabled menu item that overlaps the panel.  See bug 492960. -->
       <menupopup id="downloadsContextMenu"
                  onpopupshown="DownloadsView.onContextPopupShown(event);"
                  onpopuphidden="DownloadsView.onContextPopupHidden(event);"
                  class="download-state">
         <menuitem command="downloadsCmd_pauseResume"