Backed out changeset 9933f2d4d188 (bug 1369095) for timeouts in browser_page_action_menu.js a=backout CLOSED TREE
authorWes Kocher <wkocher@mozilla.com>
Fri, 09 Jun 2017 09:19:45 -0700
changeset 411358 2abd7ace9f93d1906645a3154bcb18c24bc8d78c
parent 411357 0aa17836c076731e011948fabb1d78dcf1703c8f
child 411359 d5c96e74c92039b8d34cf467b60c4e7e32b44858
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 out9933f2d4d1885c5b730fb5d5a5103d77219be57d
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 9933f2d4d188 (bug 1369095) for timeouts in browser_page_action_menu.js a=backout CLOSED TREE MozReview-Commit-ID: CErzYNn287D
browser/base/content/test/urlbar/browser_page_action_menu.js
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/base/content/test/urlbar/browser_page_action_menu.js
+++ b/browser/base/content/test/urlbar/browser_page_action_menu.js
@@ -328,25 +328,25 @@ function promiseViewShown() {
       });
     });
   });
 }
 
 function promiseViewShowing() {
   return new Promise(resolve => {
     gPanel.addEventListener("ViewShowing", (event) => {
-      resolve(event.originalTarget);
+      resolve(event.target);
     }, { once: true });
   });
 }
 
 function promiseTransitionEnd() {
   return new Promise(resolve => {
     gPanel.addEventListener("transitionend", (event) => {
-      resolve(event.originalTarget);
+      resolve(event.target);
     }, { once: true });
   });
 }
 
 function promiseSyncReady() {
   let service = Cc["@mozilla.org/weave/service;1"]
                   .getService(Components.interfaces.nsISupports)
                   .wrappedJSObject;
--- 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
@@ -351,16 +351,22 @@ photonpanelmultiview panelview {
 #appMenu-popup panelview {
   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;