Bug 1354086 - switch overflow panel to using a photonpanelmultiview, allowing webextension views to specify their own size, r?mikedeboer draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Wed, 19 Jul 2017 21:23:46 +0100
changeset 611493 53c5c7e4e9e3fd339200189169b0c9b433ca5ade
parent 610973 1b065ffd8a535a0ad4c39a912af18e948e6a42c1
child 611494 f8be0596421938f7e95f3f878806efdb4466be7f
push id69242
push usergijskruitbosch@gmail.com
push dateWed, 19 Jul 2017 20:25:10 +0000
reviewersmikedeboer
bugs1354086
milestone56.0a1
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);