Bug 1365647 - Make sure that the panel height never shrinks below the height of the main view in panelviews, like the main menu. r=Gijs
authorMike de Boer <mdeboer@mozilla.com>
Tue, 23 May 2017 12:07:50 +0200
changeset 360082 69c77ef0e94dac4361f0259a07fe2014ee6e0d4e
parent 360081 51114054d7767791d7304f763e5aa3ccf10ff556
child 360083 73035b57365cba0872c107cb7ede9c60f8d762db
push id43218
push usermdeboer@mozilla.com
push dateTue, 23 May 2017 10:12:43 +0000
treeherderautoland@69c77ef0e94d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs
bugs1365647
milestone55.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 1365647 - Make sure that the panel height never shrinks below the height of the main view in panelviews, like the main menu. r=Gijs MozReview-Commit-ID: EhjDg3Sci1y
browser/components/customizableui/PanelMultiView.jsm
--- a/browser/components/customizableui/PanelMultiView.jsm
+++ b/browser/components/customizableui/PanelMultiView.jsm
@@ -401,21 +401,30 @@ this.PanelMultiView = class {
       let previousViewNode = aPreviousView || this._currentSubView;
       let playTransition = (!!previousViewNode && previousViewNode != viewNode);
 
       let dwu, previousRect;
       if (playTransition || this.panelViews) {
         dwu = this._dwu;
         previousRect = previousViewNode.__lastKnownBoundingRect =
           dwu.getBoundsWithoutFlushing(previousViewNode);
-        if (this.panelViews && !this._mainViewWidth) {
-          this._mainViewWidth = previousRect.width;
-          let top = dwu.getBoundsWithoutFlushing(previousViewNode.firstChild).top;
-          let bottom = dwu.getBoundsWithoutFlushing(previousViewNode.lastChild).bottom;
-          this._viewVerticalPadding = previousRect.height - (bottom - top);
+        if (this.panelViews) {
+          // Here go the measures that have the same caching lifetime as the width
+          // of the main view, i.e. 'forever', during the instance lifetime.
+          if (!this._mainViewWidth) {
+            this._mainViewWidth = previousRect.width;
+            let top = dwu.getBoundsWithoutFlushing(previousViewNode.firstChild).top;
+            let bottom = dwu.getBoundsWithoutFlushing(previousViewNode.lastChild).bottom;
+            this._viewVerticalPadding = previousRect.height - (bottom - top);
+          }
+          // Here go the measures that have the same caching lifetime as the height
+          // of the main view, i.e. whilst the panel is shown and/ or visible.
+          if (!this._mainViewHeight) {
+            this._mainViewHeight = previousRect.height;
+          }
         }
       }
 
       // Emit the ViewShowing event so that the widget definition has a chance
       // to lazily populate the subview with things.
       let detail = {
         blockers: new Set(),
         addBlocker(aPromise) {
@@ -486,17 +495,17 @@ this.PanelMultiView = class {
         // the panel's not even open.
         if (this._panel.state != "open") {
           onTransitionEnd();
           return;
         }
 
         if (aAnchor)
           aAnchor.setAttribute("open", true);
-        this._viewContainer.style.height = previousRect.height + "px";
+        this._viewContainer.style.height = Math.max(previousRect.height, this._mainViewHeight) + "px";
         this._viewContainer.style.width = previousRect.width + "px";
 
         this._transitioning = true;
         this._viewContainer.setAttribute("transition-reverse", reverse);
         let nodeToAnimate = reverse ? previousViewNode : viewNode;
 
         if (!reverse) {
           // We set the margin here to make sure the view is positioned next
@@ -533,67 +542,70 @@ this.PanelMultiView = class {
               viewRect.height = [viewNode.header, ...viewNode.children].reduce((acc, node) => {
                 return acc + dwu.getBoundsWithoutFlushing(node).height;
               }, this._viewVerticalPadding);
             }
           }
 
           // Set the viewContainer dimensions to make sure only the current view
           // is visible.
-          this._viewContainer.style.height = viewRect.height + "px";
+          this._viewContainer.style.height = Math.max(viewRect.height, this._mainViewHeight) + "px";
           this._viewContainer.style.width = viewRect.width + "px";
 
           // The 'magic' part: build up the amount of pixels to move right or left.
           let moveToLeft = (this._dir == "rtl" && !reverse) || (this._dir == "ltr" && reverse);
           let movementX = reverse ? viewRect.width : previousRect.width;
           let moveX = (moveToLeft ? "" : "-") + movementX;
           nodeToAnimate.style.transform = "translateX(" + moveX + "px)";
           // We're setting the width property to prevent flickering during the
           // sliding animation with smaller views.
           nodeToAnimate.style.width = viewRect.width + "px";
 
           let listener;
-          let seen = 0;
           this._viewContainer.addEventListener("transitionend", listener = ev => {
-            if (ev.target == this._viewContainer && ev.propertyName == "height") {
-              // Myeah, panel layout auto-resizing is a funky thing. We'll wait
-              // another few milliseconds to remove the width and height 'fixtures',
-              // to be sure we don't flicker annoyingly.
-              // NB: HACK! Bug 1363756 is there to fix this.
-              window.setTimeout(() => {
-                this._viewContainer.style.removeProperty("height");
-                this._viewContainer.style.removeProperty("width");
-              }, 500);
-              ++seen;
-            } else if (ev.target == nodeToAnimate && ev.propertyName == "transform") {
-              onTransitionEnd();
-              this._transitioning = false;
-              this._resetKeyNavigation(previousViewNode);
+            // It's quite common that `height` on the view container doesn't need
+            // to transition, so we make sure to do all the work on the transform
+            // transition-end, because that is guaranteed to happen.
+            if (ev.target != nodeToAnimate || ev.propertyName != "transform")
+              return;
+
+            this._viewContainer.removeEventListener("transitionend", listener);
+            onTransitionEnd();
+            this._transitioning = false;
+            this._resetKeyNavigation(previousViewNode);
 
-              // Take another breather, just like before, to wait for the 'current'
-              // attribute removal to take effect. This prevents a flicker.
-              // The cleanup we do doesn't affect the display anymore, so we're not
-              // too fussed about the timing here.
-              window.addEventListener("MozAfterPaint", () => {
-                nodeToAnimate.style.removeProperty("border-inline-start");
-                nodeToAnimate.style.removeProperty("transition");
-                nodeToAnimate.style.removeProperty("transform");
-                nodeToAnimate.style.removeProperty("width");
+            // Myeah, panel layout auto-resizing is a funky thing. We'll wait
+            // another few milliseconds to remove the width and height 'fixtures',
+            // to be sure we don't flicker annoyingly.
+            // NB: HACK! Bug 1363756 is there to fix this.
+            window.setTimeout(() => {
+              // Only remove the height when the view is larger than the main
+              // view, otherwise it'll snap back to its own height.
+              if (viewRect.height > this._mainViewHeight)
+                this._viewContainer.style.removeProperty("height");
+              this._viewContainer.style.removeProperty("width");
+            }, 500);
 
-                if (!reverse)
-                  viewNode.style.removeProperty("margin-inline-start");
-                if (aAnchor)
-                  aAnchor.removeAttribute("open");
+            // Take another breather, just like before, to wait for the 'current'
+            // attribute removal to take effect. This prevents a flicker.
+            // The cleanup we do doesn't affect the display anymore, so we're not
+            // too fussed about the timing here.
+            window.addEventListener("MozAfterPaint", () => {
+              nodeToAnimate.style.removeProperty("border-inline-start");
+              nodeToAnimate.style.removeProperty("transition");
+              nodeToAnimate.style.removeProperty("transform");
+              nodeToAnimate.style.removeProperty("width");
 
-                this._viewContainer.removeAttribute("transition-reverse");
-              }, { once: true });
-              ++seen;
-            }
-            if (seen == 2)
-              this._viewContainer.removeEventListener("transitionend", listener);
+              if (!reverse)
+                viewNode.style.removeProperty("margin-inline-start");
+              if (aAnchor)
+                aAnchor.removeAttribute("open");
+
+              this._viewContainer.removeAttribute("transition-reverse");
+            }, { once: true });
           });
         }, { once: true });
       } else if (!this.panelViews) {
         this._shiftMainView(aAnchor);
 
         this._mainViewHeight = this._viewStack.clientHeight;
 
         let newHeight = this._heightOfSubview(viewNode, this._subViews);
@@ -715,16 +727,17 @@ this.PanelMultiView = class {
         this._mainView.style.removeProperty("height");
         this.showMainView();
         if (!this.panelViews) {
           this._mainViewObserver.disconnect();
         } else {
           this.window.removeEventListener("keydown", this);
           this._panel.removeEventListener("mousemove", this);
           this._resetKeyNavigation();
+          this._mainViewHeight = 0;
         }
         break;
     }
   }
 
   /**
    * Allow for navigating subview buttons using the arrow keys and the Enter key.
    * The Up and Down keys can be used to navigate the list up and down and the