Bug 1424264 - Part 1 - Always update min-width and max-width before showing views. r=Gijs draft
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Fri, 26 Jan 2018 14:05:11 +0000
changeset 747614 fba009a10b2b920e0da2c1d8b8dc57b99697bea5
parent 747260 663987f8bdd98a67581f76956cc7051ea94c460a
child 747615 8d989e9ce8fd973fb31d57d7085a1c8c2ce397d6
push id96955
push userpaolo.mozmail@amadzone.org
push dateFri, 26 Jan 2018 14:22:08 +0000
reviewersGijs
bugs1424264
milestone60.0a1
Bug 1424264 - Part 1 - Always update min-width and max-width before showing views. r=Gijs This also removes some unneeded checks for variables that are now never false. MozReview-Commit-ID: KVVwARLrpYO
browser/components/customizableui/PanelMultiView.jsm
--- a/browser/components/customizableui/PanelMultiView.jsm
+++ b/browser/components/customizableui/PanelMultiView.jsm
@@ -352,53 +352,54 @@ this.PanelMultiView = class extends this
         this._viewStack.appendChild(viewNode);
       }
 
       let nextPanelView = PanelView.forNode(viewNode);
       this.knownViews.add(nextPanelView);
 
       viewNode.panelMultiView = this.node;
 
-      let reverse = !!aPreviousView;
-      if (!reverse) {
-        nextPanelView.headerText = viewNode.getAttribute("title") ||
-                                   (aAnchor && aAnchor.getAttribute("label"));
-      }
-
       let previousViewNode = aPreviousView || this._currentSubView;
       // If the panelview to show is the same as the previous one, the 'ViewShowing'
       // event has already been dispatched. Don't do it twice.
       let showingSameView = viewNode == previousViewNode;
-      let playTransition = (!!previousViewNode && !showingSameView && this._panel.state == "open");
-      let isMainView = viewNode.id == this._mainViewId;
 
       let previousRect = previousViewNode.__lastKnownBoundingRect =
           this._dwu.getBoundsWithoutFlushing(previousViewNode);
       // Cache the measures that have the same caching lifetime as the width
       // or height of the main view, i.e. whilst the panel is shown and/ or
       // visible.
       if (!this._mainViewWidth) {
         this._mainViewWidth = previousRect.width;
       }
       if (!this._mainViewHeight) {
         this._mainViewHeight = previousRect.height;
         this._viewContainer.style.minHeight = this._mainViewHeight + "px";
       }
 
       this._viewShowing = viewNode;
-      // Because the 'mainview' attribute may be out-of-sync, due to view node
-      // reparenting in combination with ephemeral PanelMultiView instances,
-      // this is the best place to correct it (just before showing).
-      nextPanelView.mainview = isMainView;
+
+      let reverse = !!aPreviousView;
+      if (!reverse) {
+        // We are opening a new view, either because we are navigating forward
+        // or because we are showing the main view. Some properties of the view
+        // may vary between panels, so we make sure to update them every time.
+        // Firstly, make sure that the header matches how the view was opened.
+        nextPanelView.headerText = viewNode.getAttribute("title") ||
+                                   (aAnchor && aAnchor.getAttribute("label"));
+        // The main view of a panel can be a subview in another one.
+        let isMainView = viewNode.id == this._mainViewId;
+        nextPanelView.mainview = isMainView;
+        // The constrained width of subviews may also vary between panels.
+        nextPanelView.minMaxWidth = isMainView ? 0 : this._mainViewWidth;
+      }
 
       if (aAnchor) {
         viewNode.classList.add("PanelUI-subView");
       }
-      if (!isMainView && this._mainViewWidth)
-        viewNode.style.maxWidth = viewNode.style.minWidth = this._mainViewWidth + "px";
 
       if (!showingSameView || !viewNode.hasAttribute("current")) {
         // Emit the ViewShowing event so that the widget definition has a chance
         // to lazily populate the subview with things or perhaps even cancel this
         // whole operation.
         let detail = {
           blockers: new Set(),
           addBlocker(promise) {
@@ -420,17 +421,17 @@ this.PanelMultiView = class extends this
           this._viewShowing = null;
           return;
         }
       }
 
       // Now we have to transition the panel. If we've got an older transition
       // still running, make sure to clean it up.
       await this._cleanupTransitionPhase();
-      if (playTransition) {
+      if (!showingSameView && this._panel.state == "open") {
         await this._transitionViews(previousViewNode, viewNode, reverse, previousRect, aAnchor);
         nextPanelView.focusSelectedElement();
       } else {
         this.hideAllViewsExcept(nextPanelView);
       }
     })().catch(e => Cu.reportError(e));
     return this._currentShowPromise;
   }
@@ -755,18 +756,16 @@ this.PanelMultiView = class extends this
         // mid-transition, which disrupts our state:
         this._viewShowing = null;
         this._transitioning = false;
         this.node.removeAttribute("panelopen");
         this.showMainView();
         for (let panelView of this._viewStack.children) {
           if (panelView.nodeName != "children") {
             panelView.__lastKnownBoundingRect = null;
-            panelView.style.removeProperty("min-width");
-            panelView.style.removeProperty("max-width");
           }
         }
         this.window.removeEventListener("keydown", this);
         this._panel.removeEventListener("mousemove", this);
         this.openViews.forEach(panelView => panelView.clearNavigation());
         this.openViews = [];
 
         // Clear the main view size caches. The dimensions could be different
@@ -812,16 +811,30 @@ this.PanelView = class extends this.Asso
       }
     } else if (this.node.hasAttribute("current")) {
       this.dispatchCustomEvent("ViewHiding");
       this.node.removeAttribute("current");
     }
   }
 
   /**
+   * Constrains the width of this view using the "min-width" and "max-width"
+   * styles. Setting this to zero removes the constraints.
+   */
+  set minMaxWidth(value) {
+    let style = this.node.style;
+    if (value) {
+      style.minWidth = style.maxWidth = value + "px";
+    } else {
+      style.removeProperty("min-width");
+      style.removeProperty("max-width");
+    }
+  }
+
+  /**
    * Adds a header with the given title, or removes it if the title is empty.
    */
   set headerText(value) {
     // If the header already exists, update or remove it as requested.
     let header = this.node.firstChild;
     if (header && header.classList.contains("panel-header")) {
       if (value) {
         header.querySelector("label").setAttribute("value", value);