Backed out changeset 8a1615be322c (bug 1369095) for failing browser-chrome's browser_page_action_menu.js with "page-action-multiView" == "page-action-sendToDeviceView". r=backout
authorSebastian Hengst <archaeopteryx@coole-files.de>
Thu, 08 Jun 2017 19:14:40 +0200
changeset 411156 58a93f7d6b20b5809ed14294eabccd825aeb174f
parent 411155 eaef7cd5e2884314fb81c70d1b9576772a38e415
child 411157 f8d48ac82e7b06c803558ff5086882f2e122783a
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1369095
milestone55.0a1
backs out8a1615be322cb623d5f4e1dd7d85aae1c6c6b18c
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
Backed out changeset 8a1615be322c (bug 1369095) for failing browser-chrome's browser_page_action_menu.js with "page-action-multiView" == "page-action-sendToDeviceView". r=backout
browser/components/customizableui/PanelMultiView.jsm
browser/components/customizableui/content/panelUI.css
browser/components/customizableui/content/panelUI.xml
browser/themes/shared/customizableui/panelUI.inc.css
--- a/browser/components/customizableui/PanelMultiView.jsm
+++ b/browser/components/customizableui/PanelMultiView.jsm
@@ -247,18 +247,16 @@ this.PanelMultiView = class {
     this._viewContainer =
       document.getAnonymousElementByAttribute(this.node, "anonid", "viewContainer");
     this._mainViewContainer =
       document.getAnonymousElementByAttribute(this.node, "anonid", "mainViewContainer");
     this._subViews =
       document.getAnonymousElementByAttribute(this.node, "anonid", "subViews");
     this._viewStack =
       document.getAnonymousElementByAttribute(this.node, "anonid", "viewStack");
-    this._offscreenViewStack =
-      document.getAnonymousElementByAttribute(this.node, "anonid", "offscreenViewStack");
 
     XPCOMUtils.defineLazyGetter(this, "_panelViewCache", () => {
       let viewCacheId = this.node.getAttribute("viewCacheId");
       return viewCacheId ? document.getElementById(viewCacheId) : null;
     });
 
     this._panel.addEventListener("popupshowing", this);
     this._panel.addEventListener("popuphidden", this);
@@ -440,17 +438,16 @@ this.PanelMultiView = class {
             let top = dwu.getBoundsWithoutFlushing(previousViewNode.firstChild || previousViewNode).top;
             let bottom = dwu.getBoundsWithoutFlushing(previousViewNode.lastChild || previousViewNode).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;
-            this._viewContainer.style.minHeight = this._mainViewHeight + "px";
           }
         }
       }
 
       // Emit the ViewShowing event so that the widget definition has a chance
       // to lazily populate the subview with things.
       let detail = {
         blockers: new Set(),
@@ -526,160 +523,138 @@ this.PanelMultiView = class {
         // the panel's not even open.
         if (this._panel.state != "open") {
           onTransitionEnd();
           return;
         }
 
         if (aAnchor)
           aAnchor.setAttribute("open", true);
-
-        // Set the viewContainer dimensions to make sure only the current view
-        // is visible.
         this._viewContainer.style.height = Math.max(previousRect.height, this._mainViewHeight) + "px";
         this._viewContainer.style.width = previousRect.width + "px";
-        // Lock the dimensions of the window that hosts the popup panel.
-        let rect = this._panel.popupBoxObject.getOuterScreenRect();
-        this._panel.setAttribute("width", rect.width);
-        this._panel.setAttribute("height", rect.height);
+
+        this._transitioning = true;
+        if (this._autoResizeWorkaroundTimer)
+          window.clearTimeout(this._autoResizeWorkaroundTimer);
+        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
+          // to the view that is currently visible. The animation is taken
+          // care of by transitioning the `transform: translateX()` property
+          // instead.
+          // Once the transition finished, we clean both properties up.
+          nodeToAnimate.style.marginInlineStart = previousRect.width + "px";
+        }
 
-        this._viewBoundsOffscreen(viewNode, viewRect => {
-          this._transitioning = true;
-          if (this._autoResizeWorkaroundTimer)
-            window.clearTimeout(this._autoResizeWorkaroundTimer);
-          this._viewContainer.setAttribute("transition-reverse", reverse);
-          let nodeToAnimate = reverse ? previousViewNode : viewNode;
+        // Set the transition style and listen for its end to clean up and
+        // make sure the box sizing becomes dynamic again.
+        // Somehow, putting these properties in PanelUI.css doesn't work for
+        // newly shown nodes in a XUL parent node.
+        nodeToAnimate.style.transition = "transform ease-" + (reverse ? "in" : "out") +
+          " var(--panelui-subview-transition-duration)";
+        nodeToAnimate.style.willChange = "transform";
+        nodeToAnimate.style.borderInlineStart = "1px solid var(--panel-separator-color)";
 
-          if (!reverse) {
-            // We set the margin here to make sure the view is positioned next
-            // to the view that is currently visible. The animation is taken
-            // care of by transitioning the `transform: translateX()` property
-            // instead.
-            // Once the transition finished, we clean both properties up.
-            nodeToAnimate.style.marginInlineStart = previousRect.width + "px";
+        // Wait until after the first paint to ensure setting 'current=true'
+        // has taken full effect; once both views are visible, we want to
+        // correctly measure rects using `dwu.getBoundsWithoutFlushing`.
+        window.addEventListener("MozAfterPaint", () => {
+          let viewRect = viewNode.__lastKnownBoundingRect;
+          if (!viewRect) {
+            viewRect = dwu.getBoundsWithoutFlushing(viewNode);
+            if (!reverse) {
+              // When showing two nodes at the same time (one partly out of view,
+              // but that doesn't seem to make a difference in this case) inside
+              // a XUL node container, the flexible box layout on the vertical
+              // axis gets confused. I.e. it lies.
+              // So what we need to resort to here is count the height of each
+              // individual child element of the view.
+              viewRect.height = [viewNode.header, ...viewNode.children].reduce((acc, node) => {
+                return acc + dwu.getBoundsWithoutFlushing(node).height;
+              }, this._viewVerticalPadding);
+            }
           }
 
-          // Set the transition style and listen for its end to clean up and
-          // make sure the box sizing becomes dynamic again.
-          // Somehow, putting these properties in PanelUI.css doesn't work for
-          // newly shown nodes in a XUL parent node.
-          nodeToAnimate.style.transition = "transform ease-" + (reverse ? "in" : "out") +
-            " var(--panelui-subview-transition-duration)";
-          nodeToAnimate.style.willChange = "transform";
-          nodeToAnimate.style.borderInlineStart = "1px solid var(--panel-separator-color)";
+          // Set the viewContainer dimensions to make sure only the current view
+          // is visible.
+          this._viewContainer.style.height = Math.max(viewRect.height, this._mainViewHeight) + "px";
+          this._viewContainer.style.width = viewRect.width + "px";
 
-          // Wait until after the first paint to ensure setting 'current=true'
-          // has taken full effect; once both views are visible, we want to
-          // correctly measure rects using `dwu.getBoundsWithoutFlushing`.
-          window.addEventListener("MozAfterPaint", () => {
-            // Now set the viewContainer dimensions to that of the new view, which
-            // kicks of the height animation.
-            this._viewContainer.style.height = Math.max(viewRect.height, this._mainViewHeight) + "px";
-            this._viewContainer.style.width = viewRect.width + "px";
-            this._panel.removeAttribute("width");
-            this._panel.removeAttribute("height");
+          // 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";
 
-            // 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;
+          this._viewContainer.addEventListener("transitionend", listener = ev => {
+            // 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;
 
-            let listener;
-            this._viewContainer.addEventListener("transitionend", listener = ev => {
-              // 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);
 
-              this._viewContainer.removeEventListener("transitionend", listener);
-              onTransitionEnd();
-              this._transitioning = false;
-              this._resetKeyNavigation(previousViewNode);
-
-              // 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.
-              this._autoResizeWorkaroundTimer = window.setTimeout(() => {
+            // 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.
+            this._autoResizeWorkaroundTimer = 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 (viewNode == this._mainView || viewRect.height > this._mainViewHeight)
                 this._viewContainer.style.removeProperty("height");
-                this._viewContainer.style.removeProperty("width");
-              }, 500);
+              this._viewContainer.style.removeProperty("width");
+            }, 500);
 
-              // 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");
+            // 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");
 
-                if (!reverse)
-                  viewNode.style.removeProperty("margin-inline-start");
-                if (aAnchor)
-                  aAnchor.removeAttribute("open");
-
-                this._viewContainer.removeAttribute("transition-reverse");
+              if (!reverse)
+                viewNode.style.removeProperty("margin-inline-start");
+              if (aAnchor)
+                aAnchor.removeAttribute("open");
 
-                evt = new window.CustomEvent("ViewShown", { bubbles: true, cancelable: false });
-                viewNode.dispatchEvent(evt);
-              }, { once: true });
-            });
-          }, { once: true });
-        });
+              this._viewContainer.removeAttribute("transition-reverse");
+
+              evt = new window.CustomEvent("ViewShown", { bubbles: true, cancelable: false });
+              viewNode.dispatchEvent(evt);
+            }, { once: true });
+          });
+        }, { once: true });
       } else if (!this.panelViews) {
         this._transitionHeight(() => {
           viewNode.setAttribute("current", true);
           this.node.setAttribute("viewtype", "subview");
           // Now that the subview is visible, we can check the height of the
           // description elements it contains.
           this.descriptionHeightWorkaround(viewNode);
         });
         this._shiftMainView(aAnchor);
       }
     })().catch(e => Cu.reportError(e));
   }
 
   /**
-   * Calculate the correct bounds of a panelview node offscreen to minimize the
-   * amount of paint flashing and keep the stack vs panel layouts from interfering.
-   *
-   * @param {panelview} viewNode Node to measure the bounds of.
-   * @param {Function}  callback Called when we got the measurements in and pass
-   *                             them on as its first argument.
-   */
-  _viewBoundsOffscreen(viewNode, callback) {
-    if (viewNode.__lastKnownBoundingRect) {
-      callback(viewNode.__lastKnownBoundingRect);
-      return;
-    }
-
-    let oldSibling = viewNode.nextSibling || null;
-    this._offscreenViewStack.appendChild(viewNode);
-
-    this.window.addEventListener("MozAfterPaint", () => {
-      let viewRect = this._dwu.getBoundsWithoutFlushing(viewNode);
-
-      try {
-        this._viewStack.insertBefore(viewNode, oldSibling);
-      } catch (ex) {
-        this._viewStack.appendChild(viewNode);
-      }
-
-      callback(viewRect);
-    }, { once: true });
-  }
-
-  /**
    * Applies the height transition for which <panelmultiview> is designed.
    *
    * The height transition involves two elements, the viewContainer and its only
    * immediate child the viewStack. In order for this to work correctly, the
    * viewContainer must have "overflow: hidden;" and the two elements must have
    * no margins or padding. This means that the height of the viewStack is never
    * limited by the viewContainer, but when the height of the container is not
    * constrained it matches the height of the viewStack.
--- a/browser/components/customizableui/content/panelUI.css
+++ b/browser/components/customizableui/content/panelUI.css
@@ -43,21 +43,9 @@
 photonpanelmultiview > .panel-viewcontainer > .panel-viewstack {
   overflow: visible;
 }
 
 photonpanelmultiview[transitioning] {
   pointer-events: none;
 }
 
-.panel-viewcontainer.offscreen {
-  position: absolute;
-  top: 100000px;
-  left: 100000px;
-}
-
-.panel-viewcontainer.offscreen,
-.panel-viewcontainer.offscreen > .panel-viewstack {
-  margin: 0;
-  padding: 0;
-}
-
 /* END photon adjustments */
--- a/browser/components/customizableui/content/panelUI.xml
+++ b/browser/components/customizableui/content/panelUI.xml
@@ -53,19 +53,16 @@
 
   <binding id="photonpanelmultiview" extends="chrome://browser/content/customizableui/panelUI.xml#panelmultiview">
     <content>
       <xul:box anonid="viewContainer" class="panel-viewcontainer" xbl:inherits="panelopen,transitioning">
         <xul:stack anonid="viewStack" xbl:inherits="transitioning" class="panel-viewstack">
           <children includes="panelview"/>
         </xul:stack>
       </xul:box>
-      <xul:box class="panel-viewcontainer offscreen">
-        <xul:box anonid="offscreenViewStack" class="panel-viewstack"/>
-      </xul:box>
     </content>
   </binding>
 
   <binding id="panelview">
     <content>
       <xul:box class="panel-header" anonid="header">
         <xul:toolbarbutton anonid="back"
                            class="subviewbutton subviewbutton-iconic subviewbutton-back"
--- a/browser/themes/shared/customizableui/panelUI.inc.css
+++ b/browser/themes/shared/customizableui/panelUI.inc.css
@@ -348,16 +348,22 @@ photonpanelmultiview panelview {
   padding: 6px 0;
   min-width: @menuPanelWidth@;
 }
 
 photonpanelmultiview panelview[title] {
   padding-top: 0;
 }
 
+photonpanelmultiview .panel-subview-body {
+  /*XXXmikedeboer this flex is unnecessary, so I unset it for our case. It might
+                  break other panels, though, so I refrain from removing it above. */
+  -moz-box-flex: unset;
+}
+
 /* END photonpanelview adjustments */
 
 .cui-widget-panel.cui-widget-panelWithFooter > .panel-arrowcontainer > .panel-arrowcontent {
   padding-bottom: 0;
 }
 
 #PanelUI-contents {
   display: block;