Bug 1354086 - switch overflow panel to using a photonpanelmultiview, allowing webextension views to specify their own size, r=mikedeboer
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Wed, 19 Jul 2017 21:23:46 +0100
changeset 418483 5ca4811f78368b4f5722dae3801a0a9370e89277
parent 418482 b626aca18e669be6957857b06779cd8b3bb8dbb7
child 418484 4a279604f7ba3e11599f91a270f6c52d4ab299f5
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmikedeboer
bugs1354086
milestone56.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 1354086 - switch overflow panel to using a photonpanelmultiview, allowing webextension views to specify their own size, r=mikedeboer MozReview-Commit-ID: 1uHEKXsO8vh
browser/components/customizableui/CustomizableUI.jsm
browser/components/customizableui/PanelMultiView.jsm
browser/components/customizableui/content/panelUI.inc.xul
browser/components/extensions/ExtensionPopups.jsm
browser/components/extensions/ext-browserAction.js
browser/components/extensions/test/browser/browser_ext_browserAction_popup_resize.js
browser/components/extensions/test/browser/head.js
--- a/browser/components/customizableui/CustomizableUI.jsm
+++ b/browser/components/customizableui/CustomizableUI.jsm
@@ -4204,17 +4204,17 @@ OverflowableToolbar.prototype = {
 
   show() {
     if (this._panel.state == "open") {
       return Promise.resolve();
     }
     return new Promise(resolve => {
       let doc = this._panel.ownerDocument;
       this._panel.hidden = false;
-      let photonView = this._panel.querySelector("panelmultiview");
+      let photonView = this._panel.querySelector("photonpanelmultiview");
       let contextMenu;
       if (photonView) {
         let mainViewId = photonView.getAttribute("mainViewId");
         let mainView = doc.getElementById(mainViewId);
         contextMenu = doc.getElementById(mainView.getAttribute("context"));
       } else {
         contextMenu = doc.getElementById(this._panel.getAttribute("context"));
       }
--- a/browser/components/customizableui/PanelMultiView.jsm
+++ b/browser/components/customizableui/PanelMultiView.jsm
@@ -410,16 +410,17 @@ this.PanelMultiView = class {
   showMainView() {
     if (this.showingSubView) {
       let viewNode = this._currentSubView;
       let evt = new this.window.CustomEvent("ViewHiding", { bubbles: true, cancelable: true });
       viewNode.dispatchEvent(evt);
       if (this.panelViews) {
         viewNode.removeAttribute("current");
         this.showSubView(this._mainViewId);
+        this.node.setAttribute("viewtype", "main");
       } else {
         this._transitionHeight(() => {
           viewNode.removeAttribute("current");
           this._currentSubView = null;
           this.node.setAttribute("viewtype", "main");
         });
       }
     }
@@ -508,21 +509,21 @@ this.PanelMultiView = class {
           let results = await Promise.all(detail.blockers);
           cancel = cancel || results.some(val => val === false);
         } catch (e) {
           Cu.reportError(e);
           cancel = true;
         }
       }
 
+      this._viewShowing = null;
       if (cancel) {
         return;
       }
 
-      this._viewShowing = null;
       this._currentSubView = viewNode;
       viewNode.setAttribute("current", true);
       if (this.panelViews) {
         this.node.setAttribute("viewtype", "subview");
         if (!playTransition)
           this.descriptionHeightWorkaround(viewNode);
       }
 
@@ -562,17 +563,17 @@ this.PanelMultiView = class {
         // 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._viewBoundsOffscreen(viewNode, viewRect => {
+        this._viewBoundsOffscreen(viewNode, previousRect, viewRect => {
           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
@@ -672,25 +673,40 @@ this.PanelMultiView = class {
     })().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 {Rect}      previousRect Rect representing the previous view
+   *                                 (used to fill in any blanks).
    * @param {Function}  callback Called when we got the measurements in and pass
    *                             them on as its first argument.
    */
-  _viewBoundsOffscreen(viewNode, callback) {
+  _viewBoundsOffscreen(viewNode, previousRect, callback) {
     if (viewNode.__lastKnownBoundingRect) {
       callback(viewNode.__lastKnownBoundingRect);
       return;
     }
 
+    if (viewNode.customRectGetter) {
+      // Can't use Object.assign directly with a DOM Rect object because its properties
+      // aren't enumerable.
+      let {height, width} = previousRect;
+      let rect = Object.assign({height, width}, viewNode.customRectGetter());
+      let {header} = viewNode;
+      if (header) {
+        rect.height += this._dwu.getBoundsWithoutFlushing(header).height;
+      }
+      callback(rect);
+      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);
@@ -904,16 +920,20 @@ this.PanelMultiView = class {
         }
         break;
       case "popupshown":
         // Now that the main view is visible, we can check the height of the
         // description elements it contains.
         this.descriptionHeightWorkaround();
         break;
       case "popuphidden":
+        // WebExtensions consumers can hide the popup from viewshowing, or
+        // mid-transition, which disrupts our state:
+        this._viewShowing = null;
+        this._transitioning = false;
         this.node.removeAttribute("panelopen");
         this.showMainView();
         if (this.panelViews) {
           for (let panelView of this._viewStack.children) {
             if (panelView.nodeName != "children") {
               panelView.style.removeProperty("min-width");
               panelView.style.removeProperty("max-width");
             }
--- a/browser/components/customizableui/content/panelUI.inc.xul
+++ b/browser/components/customizableui/content/panelUI.inc.xul
@@ -401,30 +401,30 @@
        type="arrow"
        noautofocus="true"
        position="bottomcenter topright"
 #ifndef MOZ_PHOTON_THEME
        context="toolbar-context-menu"
 #endif
        hidden="true">
 #ifdef MOZ_PHOTON_THEME
-  <panelmultiview mainViewId="widget-overflow-mainView">
+  <photonpanelmultiview mainViewId="widget-overflow-mainView">
     <panelview id="widget-overflow-mainView"
                context="toolbar-context-menu">
 #endif
       <vbox id="widget-overflow-scroller">
         <vbox id="widget-overflow-list" class="widget-overflow-list"
               overflowfortoolbar="nav-bar"/>
         <toolbarseparator id="widget-overflow-fixed-separator" hidden="true"/>
         <vbox id="widget-overflow-fixed-list" class="widget-overflow-list" hidden="true"
               emptylabel="&customizeMode.emptyOverflowList.description;"/>
       </vbox>
 #ifdef MOZ_PHOTON_THEME
     </panelview>
-  </panelmultiview>
+  </photonpanelmultiview>
 #endif
 </panel>
 
 <panel id="customization-tipPanel"
        type="arrow"
        flip="none"
        side="left"
        position="leftcenter topright"
--- a/browser/components/extensions/ExtensionPopups.jsm
+++ b/browser/components/extensions/ExtensionPopups.jsm
@@ -116,16 +116,17 @@ class BasePopup {
       if (this.browser) {
         this.destroyBrowser(this.browser, true);
         this.browser.remove();
       }
 
       if (this.viewNode) {
         this.viewNode.removeEventListener(this.DESTROY_EVENT, this);
         this.viewNode.style.maxHeight = "";
+        delete this.viewNode.customRectGetter;
       }
 
       let {panel} = this;
       if (panel) {
         panel.style.removeProperty("--arrowpanel-background");
         panel.style.removeProperty("--panel-arrow-image-vertical");
         panel.removeAttribute("remote");
       }
@@ -321,16 +322,19 @@ class BasePopup {
 
       // Set a maximum height on the <panelview> element to our preferred
       // maximum height, so that the PanelUI resizing code can make an accurate
       // calculation. If we don't do this, the flex sizing logic will prevent us
       // from ever reporting a preferred size smaller than the height currently
       // available to us in the panel.
       height = Math.max(height, this.viewHeight);
       this.viewNode.style.maxHeight = `${height}px`;
+      // Used by the panelmultiview code to figure out sizing without reparenting
+      // (which would destroy the browser and break us).
+      this.lastCalculatedInViewHeight = height;
     } else {
       this.browser.style.width = `${width}px`;
       this.browser.style.minWidth = `${width}px`;
       this.browser.style.height = `${height}px`;
       this.browser.style.minHeight = `${height}px`;
     }
 
     let event = new this.window.CustomEvent("WebExtPopupResized", {detail});
@@ -514,16 +518,20 @@ class ViewPopup extends BasePopup {
 
     this.browser.swapDocShells(browser);
     this.destroyBrowser(browser);
 
     if (this.dimensions && !this.fixedWidth) {
       this.resizeBrowser(this.dimensions);
     }
 
+    this.viewNode.customRectGetter = () => {
+      return {height: this.lastCalculatedInViewHeight || this.viewHeight};
+    };
+
     this.tempPanel.remove();
     this.tempPanel = null;
 
     this.shown = true;
 
     if (this.destroyed) {
       this.closePopup();
       this.destroy();
--- a/browser/components/extensions/ext-browserAction.js
+++ b/browser/components/extensions/ext-browserAction.js
@@ -176,20 +176,16 @@ this.browserAction = class extends Exten
         let popupURL = this.getProperty(tab, "popup");
         this.tabManager.addActiveTabPermission(tab);
 
         // Popups are shown only if a popup URL is defined; otherwise
         // a "click" event is dispatched. This is done for compatibility with the
         // Google Chrome onClicked extension API.
         if (popupURL) {
           try {
-            // FIXME: The line below needs to change eventually, but for now:
-            // ensure the view is _always_ visible _before_ `popup.attach()` is
-            // called. PanelMultiView.jsm dictates different behavior.
-            event.target.setAttribute("current", true);
             let popup = this.getPopup(document.defaultView, popupURL);
             let attachPromise = popup.attach(event.target);
             event.detail.addBlocker(attachPromise);
             await attachPromise;
             TelemetryStopwatch.finish(POPUP_OPEN_MS_HISTOGRAM, this);
             if (this.eventQueue.length) {
               let histogram = Services.telemetry.getHistogramById(POPUP_RESULT_HISTOGRAM);
               histogram.add("popupShown");
--- a/browser/components/extensions/test/browser/browser_ext_browserAction_popup_resize.js
+++ b/browser/components/extensions/test/browser/browser_ext_browserAction_popup_resize.js
@@ -177,16 +177,22 @@ async function testPopupSize(standardsMo
 
       let panelTop = browserWin.mozInnerScreenY + panelRect.top;
       ok(panelTop >= browserWin.screen.availTop, `Top of popup should be on-screen. (${panelTop} >= ${browserWin.screen.availTop})`);
     }
   };
 
   await awaitBrowserLoaded(browser);
 
+  let panelview = browser.closest("panelview");
+  // Need to wait first for the forced panel width and for the panelview's border to be gone,
+  // then for layout to happen again. Otherwise the removal of the border between views in the
+  // panelmultiview trips up our width checking causing it to be off-by-one.
+  await BrowserTestUtils.waitForCondition(() => (!panel.hasAttribute("width") && (!panelview || !panelview.style.borderInlineStart)));
+  await promiseAnimationFrame(browserWin);
   // Wait long enough to make sure the initial resize debouncing timer has
   // expired.
   await delay(100);
 
   let dims = await promiseContentDimensions(browser);
 
   is(dims.isStandards, standardsMode, "Document has the expected compat mode");
 
--- a/browser/components/extensions/test/browser/head.js
+++ b/browser/components/extensions/test/browser/head.js
@@ -200,17 +200,17 @@ var awaitBrowserLoaded = browser => Cont
 var awaitExtensionPanel = async function(extension, win = window, awaitLoad = true) {
   let {originalTarget: browser} = await BrowserTestUtils.waitForEvent(
     win.document, "WebExtPopupLoaded", true,
     event => event.detail.extension.id === extension.id);
 
   await Promise.all([
     promisePopupShown(getPanelForNode(browser)),
 
-    awaitLoad && awaitBrowserLoaded(browser, awaitLoad),
+    awaitLoad && awaitBrowserLoaded(browser),
   ]);
 
   return browser;
 };
 
 function getCustomizableUIPanelID() {
   return gPhotonStructure ? CustomizableUI.AREA_FIXED_OVERFLOW_PANEL
                           : CustomizableUI.AREA_PANEL;
@@ -247,18 +247,17 @@ var showBrowserAction = async function(e
   }
 };
 
 var clickBrowserAction = async function(extension, win = window) {
   await promiseAnimationFrame(win);
   await showBrowserAction(extension, win);
 
   let widget = getBrowserActionWidget(extension).forWindow(win);
-
-  EventUtils.synthesizeMouseAtCenter(widget.node, {}, win);
+  widget.node.click();
 };
 
 function closeBrowserAction(extension, win = window) {
   let group = getBrowserActionWidget(extension);
 
   let node = win.document.getElementById(group.viewId);
   CustomizableUI.hidePanelForNode(node);