Bug 1428839 - Part 2 - Ensure that views passed to showSubView are added to the list of known views without having to reset the internal _panelViews variable. r=Gijs draft
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Wed, 17 Jan 2018 15:04:18 +0000
changeset 721629 c15ad57a3644635e346953889a95531f77c466e1
parent 721628 cc4d53871cb65e2ec3aa52cfb65e03c24824d67f
child 721630 dcf9f23c173ad1aa44921b59c6e7852b826f54ce
child 722192 06afc9fd6f106a974b35249ede9dd459f6360b27
push id95901
push userpaolo.mozmail@amadzone.org
push dateWed, 17 Jan 2018 16:23:27 +0000
reviewersGijs
bugs1428839
milestone59.0a1
Bug 1428839 - Part 2 - Ensure that views passed to showSubView are added to the list of known views without having to reset the internal _panelViews variable. r=Gijs MozReview-Commit-ID: Hld45ghdduv
browser/base/content/browser-pageActions.js
browser/components/customizableui/PanelMultiView.jsm
--- a/browser/base/content/browser-pageActions.js
+++ b/browser/base/content/browser-pageActions.js
@@ -131,17 +131,16 @@ var BrowserPageActions = {
       for (let name in action.nodeAttributes) {
         buttonNode.setAttribute(name, action.nodeAttributes[name]);
       }
     }
     let panelViewNode = null;
     if (action.subview) {
       buttonNode.classList.add("subviewbutton-nav");
       panelViewNode = this._makePanelViewNodeForAction(action, false);
-      this.multiViewNode._panelViews = null;
       this.multiViewNode.appendChild(panelViewNode);
     }
     buttonNode.addEventListener("command", event => {
       this.doCommandForAction(action, event, buttonNode);
     });
     return [buttonNode, panelViewNode];
   },
 
--- a/browser/components/customizableui/PanelMultiView.jsm
+++ b/browser/components/customizableui/PanelMultiView.jsm
@@ -286,18 +286,17 @@ this.PanelMultiView = class {
     // it when transitioning between panels.
     this._dir = cs.direction;
     this.showMainView();
 
     this._showingSubView = false;
 
     // Proxy these public properties and methods, as used elsewhere by various
     // parts of the browser, to this instance.
-    ["_mainView", "ignoreMutations", "showingSubView",
-     "_panelViews"].forEach(property => {
+    ["_mainView", "ignoreMutations", "showingSubView"].forEach(property => {
       Object.defineProperty(this.node, property, {
         enumerable: true,
         get: () => this[property],
         set: (val) => this[property] = val
       });
     });
     ["goBack", "descriptionHeightWorkaround", "showMainView",
      "showSubView"].forEach(method => {
@@ -356,22 +355,16 @@ this.PanelMultiView = class {
     let subviews = Array.from(viewNodeContainer.childNodes);
     for (let subview of subviews) {
       // XBL lists the 'children' XBL element explicitly. :-(
       if (subview.nodeName != "children")
         this._panelViewCache.appendChild(subview);
     }
   }
 
-  _placeSubView(viewNode) {
-    this._viewStack.appendChild(viewNode);
-    if (!this.panelViews.includes(viewNode))
-      this.panelViews.push(viewNode);
-  }
-
   _setHeader(viewNode, titleText) {
     // If the header already exists, update or remove it as requested.
     let header = viewNode.firstChild;
     if (header && header.classList.contains("panel-header")) {
       if (titleText) {
         header.querySelector("label").setAttribute("value", titleText);
       } else {
         header.remove();
@@ -433,17 +426,17 @@ this.PanelMultiView = class {
   /**
    * Ensures that all the panelviews, that are currently part of this instance,
    * are hidden, except one specifically.
    *
    * @param {panelview} [theOne] The panelview DOM node to ensure is visible.
    *                             Optional.
    */
   hideAllViewsExcept(theOne = null) {
-    for (let panelview of this._panelViews) {
+    for (let panelview of this.panelViews) {
       // When the panelview was already reparented, don't interfere any more.
       if (panelview == theOne || !this.node || panelview.panelMultiView != this.node)
         continue;
       if (panelview.hasAttribute("current"))
         this._dispatchViewEvent(panelview, "ViewHiding");
       panelview.removeAttribute("current");
     }
 
@@ -463,24 +456,27 @@ this.PanelMultiView = class {
 
   showSubView(aViewId, aAnchor, aPreviousView) {
     this._currentShowPromise = (async () => {
       // Support passing in the node directly.
       let viewNode = typeof aViewId == "string" ? this.node.querySelector("#" + aViewId) : aViewId;
       if (!viewNode) {
         viewNode = this.document.getElementById(aViewId);
         if (viewNode) {
-          this._placeSubView(viewNode);
+          this._viewStack.appendChild(viewNode);
         } else {
           throw new Error(`Subview ${aViewId} doesn't exist!`);
         }
       } else if (viewNode.parentNode == this._panelViewCache) {
-        this._placeSubView(viewNode);
+        this._viewStack.appendChild(viewNode);
       }
 
+      if (!this.panelViews.includes(viewNode))
+        this.panelViews.push(viewNode);
+
       viewNode.panelMultiView = this.node;
 
       let reverse = !!aPreviousView;
       if (!reverse) {
         this._setHeader(viewNode, viewNode.getAttribute("title") ||
                                   (aAnchor && aAnchor.getAttribute("label")));
       }
 
@@ -866,17 +862,17 @@ this.PanelMultiView = class {
       case "keydown":
         this._keyNavigation(aEvent);
         break;
       case "mousemove":
         this._resetKeyNavigation();
         break;
       case "popupshowing": {
         this.node.setAttribute("panelopen", "true");
-        if (this.panelViews && !this.node.hasAttribute("disablekeynav")) {
+        if (!this.node.hasAttribute("disablekeynav")) {
           this.window.addEventListener("keydown", this);
           this._panel.addEventListener("mousemove", this);
         }
         break;
       }
       case "popuppositioned": {
         // When autoPosition is true, the popup window manager would attempt to re-position
         // the panel as subviews are opened and it changes size. The resulting popoppositioned