Bug 1424264 - Part 1 - Always update min-width and max-width before showing views. r=Gijs draft
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Tue, 23 Jan 2018 11:28:46 +0000
changeset 723561 b9ce4556b3479e93c8e9b80d6da3bd17a378e66a
parent 723560 abeed534bd582af7618378aa44a6b7d407667ebd
child 723562 3cc6462adf9cb763c740c62c26ec73bee5243495
push id96468
push userpaolo.mozmail@amadzone.org
push dateTue, 23 Jan 2018 15:06:51 +0000
reviewersGijs
bugs1424264
milestone59.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,52 @@ 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. We have to make sure that
+        // the view appears properly based on how it's been opened and on the
+        // size of the previous views.
+        let isMainView = viewNode.id == this._mainViewId;
+        nextPanelView.mainview = isMainView;
+        nextPanelView.minMaxWidthPx = isMainView ? 0 : this._mainViewWidth;
+        nextPanelView.headerText = viewNode.getAttribute("title") ||
+                                   (aAnchor && aAnchor.getAttribute("label"));
+      }
 
       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 +419,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 +754,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
@@ -811,16 +808,26 @@ this.PanelView = class extends this.Asso
         this.dispatchCustomEvent("ViewShown");
       }
     } else if (this.node.hasAttribute("current")) {
       this.dispatchCustomEvent("ViewHiding");
       this.node.removeAttribute("current");
     }
   }
 
+  set minMaxWidthPx(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) {