Bug 1282189: Improve resizing behavior for browser action views in menu panel. r?Gijs draft
authorKris Maglione <maglione.k@gmail.com>
Thu, 21 Jul 2016 18:17:10 -0700
changeset 392661 0acb40c0169afbbfb5fda32dff2f3a9e455c0472
parent 391031 2e3390571fdb3a1ff3d2f7f828adf67dbc237bc8
child 526376 bd5ded0270c154b1c0b0c05d62ccbe39b6cb46cd
push id24079
push usermaglione.k@gmail.com
push dateMon, 25 Jul 2016 23:37:23 +0000
reviewersGijs
bugs1282189
milestone50.0a1
Bug 1282189: Improve resizing behavior for browser action views in menu panel. r?Gijs MozReview-Commit-ID: 3SPQ1IimAY8
browser/components/extensions/ext-utils.js
browser/components/extensions/test/browser/browser_ext_browserAction_popup_resize.js
browser/themes/shared/customizableui/panelUI.inc.css
--- a/browser/components/extensions/ext-utils.js
+++ b/browser/components/extensions/ext-utils.js
@@ -70,16 +70,21 @@ class BasePopup {
       Services.scriptSecurityManager.DISALLOW_SCRIPT);
 
     this.extension = extension;
     this.popupURI = popupURI;
     this.viewNode = viewNode;
     this.browserStyle = browserStyle;
     this.window = viewNode.ownerGlobal;
 
+    this.panel = this.viewNode;
+    while (this.panel.localName != "panel") {
+      this.panel = this.panel.parentNode;
+    }
+
     this.contentReady = new Promise(resolve => {
       this._resolveContentReady = resolve;
     });
 
     this.viewNode.addEventListener(this.DESTROY_EVENT, this);
 
     this.browser = null;
     this.browserReady = this.createBrowser(viewNode, popupURI);
@@ -88,29 +93,34 @@ class BasePopup {
   destroy() {
     this.browserReady.then(() => {
       this.browser.removeEventListener("DOMWindowCreated", this, true);
       this.browser.removeEventListener("load", this, true);
       this.browser.removeEventListener("DOMTitleChanged", this, true);
       this.browser.removeEventListener("DOMWindowClose", this, true);
       this.browser.removeEventListener("MozScrolledAreaChanged", this, true);
       this.viewNode.removeEventListener(this.DESTROY_EVENT, this);
+      this.viewNode.style.maxHeight = "";
       this.browser.remove();
 
       this.browser = null;
       this.viewNode = null;
     });
   }
 
   // Returns the name of the event fired on `viewNode` when the popup is being
   // destroyed. This must be implemented by every subclass.
   get DESTROY_EVENT() {
     throw new Error("Not implemented");
   }
 
+  get fixedWidth() {
+    return false;
+  }
+
   handleEvent(event) {
     switch (event.type) {
       case this.DESTROY_EVENT:
         this.destroy();
         break;
 
       case "DOMWindowCreated":
         if (this.browserStyle && event.target === this.browser.contentDocument) {
@@ -161,25 +171,26 @@ class BasePopup {
 
   createBrowser(viewNode, popupURI) {
     let document = viewNode.ownerDocument;
     this.browser = document.createElementNS(XUL_NS, "browser");
     this.browser.setAttribute("type", "content");
     this.browser.setAttribute("disableglobalhistory", "true");
     this.browser.setAttribute("webextension-view-type", "popup");
 
+    // We only need flex sizing for the sake of the slide-in sub-views of the
+    // main menu panel, so that the browser occupies the full width of the view,
+    // and also takes up any extra height that's available to it.
+    this.browser.setAttribute("flex", "1");
+
     // Note: When using noautohide panels, the popup manager will add width and
     // height attributes to the panel, breaking our resize code, if the browser
     // starts out smaller than 30px by 10px. This isn't an issue now, but it
     // will be if and when we popup debugging.
 
-    // This overrides the content's preferred size when displayed in a
-    // fixed-size, slide-in panel.
-    this.browser.setAttribute("flex", "1");
-
     viewNode.appendChild(this.browser);
 
     return new Promise(resolve => {
       // The first load event is for about:blank.
       // We can't finish setting up the browser until the binding has fully
       // initialized. Waiting for the first load event guarantees that it has.
       let loadListener = event => {
         this.browser.removeEventListener("load", loadListener, true);
@@ -212,38 +223,86 @@ class BasePopup {
 
   _resizeBrowser() {
     this.resizeTimeout = null;
 
     if (!this.browser) {
       return;
     }
 
-    let width, height;
-    try {
-      let w = {}, h = {};
-      this.browser.docShell.contentViewer.getContentSize(w, h);
+    if (this.fixedWidth) {
+      // If we're in a fixed-width area (namely a slide-in subview of the main
+      // menu panel), we need to calculate the view height based on the
+      // preferred height of the content document's root scrollable element at the
+      // current width, rather than the complete preferred dimensions of the
+      // content window.
+
+      let doc = this.browser.contentDocument;
+      if (!doc || !doc.documentElement) {
+        return;
+      }
 
-      width = w.value / this.window.devicePixelRatio;
-      height = h.value / this.window.devicePixelRatio;
+      let root = doc.documentElement;
+      let body = doc.body;
+      if (!body || doc.compatMode == "BackCompat") {
+        // In quirks mode, the root element is used as the scroll frame, and the
+        // body lies about its scroll geometry, and returns the values for the
+        // root instead.
+        body = root;
+      }
+
+      // Compensate for any offsets (margin, padding, ...) between the scroll
+      // area of the body and the outer height of the document.
+      let getHeight = elem => elem.getBoundingClientRect(elem).height;
+      let bodyPadding = getHeight(root) - getHeight(body);
+
+      let height = Math.ceil(body.scrollHeight + bodyPadding);
+
+      // Figure out how much extra space we have on the side of the panel
+      // opposite the arrow.
+      let side = this.panel.getAttribute("side") == "top" ? "bottom" : "top";
+      let maxHeight = this.viewHeight + this.extraHeight[side];
 
-      // The width calculation is imperfect, and is often a fraction of a pixel
-      // too narrow, even after taking the ceiling, which causes lines of text
-      // to wrap.
-      width += 1;
-    } catch (e) {
-      // getContentSize can throw
-      [width, height] = [400, 400];
+      height = Math.min(height, maxHeight);
+      this.browser.style.height = `${height}px`;
+
+      // 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`;
+    } else {
+      let width, height;
+      try {
+        let w = {}, h = {};
+        this.browser.docShell.contentViewer.getContentSize(w, h);
+
+        width = w.value / this.window.devicePixelRatio;
+        height = h.value / this.window.devicePixelRatio;
+
+        // The width calculation is imperfect, and is often a fraction of a pixel
+        // too narrow, even after taking the ceiling, which causes lines of text
+        // to wrap.
+        width += 1;
+      } catch (e) {
+        // getContentSize can throw
+        [width, height] = [400, 400];
+      }
+
+      width = Math.ceil(Math.min(width, 800));
+      height = Math.ceil(Math.min(height, 600));
+
+      this.browser.style.width = `${width}px`;
+      this.browser.style.height = `${height}px`;
     }
 
-    width = Math.ceil(Math.min(width, 800));
-    height = Math.ceil(Math.min(height, 600));
-
-    this.browser.style.width = `${width}px`;
-    this.browser.style.height = `${height}px`;
+    let event = new this.window.CustomEvent("WebExtPopupResized");
+    this.browser.dispatchEvent(event);
 
     this._resolveContentReady();
   }
 }
 
 global.PanelPopup = class PanelPopup extends BasePopup {
   constructor(extension, imageNode, popupURL, browserStyle) {
     let document = imageNode.ownerDocument;
@@ -278,20 +337,44 @@ global.PanelPopup = class PanelPopup ext
       if (this.viewNode) {
         this.viewNode.hidePopup();
       }
     });
   }
 };
 
 global.ViewPopup = class ViewPopup extends BasePopup {
+  constructor(...args) {
+    super(...args);
+
+    // Store the initial height of the view, so that we never resize menu panel
+    // sub-views smaller than the initial height of the menu.
+    this.viewHeight = this.viewNode.boxObject.height;
+
+    // Calculate the extra height available on the screen above and below the
+    // menu panel. Use that to calculate the how much the sub-view may grow.
+    let popupRect = this.panel.getBoundingClientRect();
+
+    let win = this.window;
+    let popupBottom = win.mozInnerScreenY + popupRect.bottom;
+    let popupTop = win.mozInnerScreenY + popupRect.top;
+    this.extraHeight = {
+      bottom: Math.max(0, win.screen.height - popupBottom),
+      top:  Math.max(0, popupTop),
+    };
+  }
+
   get DESTROY_EVENT() {
     return "ViewHiding";
   }
 
+  get fixedWidth() {
+    return !this.viewNode.classList.contains("cui-widget-panelview");
+  }
+
   closePopup() {
     CustomizableUI.hidePanelForNode(this.viewNode);
   }
 };
 
 // Manages tab-specific context data, and dispatching tab select events
 // across all windows.
 global.TabContext = function TabContext(getDefaults, extension) {
--- a/browser/components/extensions/test/browser/browser_ext_browserAction_popup_resize.js
+++ b/browser/components/extensions/test/browser/browser_ext_browserAction_popup_resize.js
@@ -1,37 +1,57 @@
 /* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set sts=2 sw=2 et tw=80: */
 "use strict";
 
-add_task(function* testPageActionPopupResize() {
+function* openPanel(extension, win = window) {
+  clickBrowserAction(extension, win);
+
+  let {target} = yield BrowserTestUtils.waitForEvent(win.document, "load", true, (event) => {
+    return event.target.location && event.target.location.href.endsWith("popup.html");
+  });
+
+  return target.defaultView
+               .QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDocShell)
+               .chromeEventHandler;
+}
+
+function* awaitResize(browser) {
+  // Debouncing code makes this a bit racy.
+  // Try to skip the first, early resize, and catch the resize event we're
+  // looking for, but don't wait longer than a few seconds.
+
+  return Promise.race([
+    BrowserTestUtils.waitForEvent(browser, "WebExtPopupResized")
+      .then(() => BrowserTestUtils.waitForEvent(browser, "WebExtPopupResized")),
+    new Promise(resolve => setTimeout(resolve, 5000)),
+  ]);
+}
+
+add_task(function* testBrowserActionPopupResize() {
   let extension = ExtensionTestUtils.loadExtension({
     manifest: {
       "browser_action": {
         "default_popup": "popup.html",
         "browser_style": true,
       },
     },
 
     files: {
-      "popup.html": "<html><head><meta charset=\"utf-8\"></head></html>",
+      "popup.html": '<html><head><meta charset="utf-8"></head></html>',
     },
   });
 
   yield extension.startup();
 
   clickBrowserAction(extension, window);
 
-  let {target: panelDocument} = yield BrowserTestUtils.waitForEvent(document, "load", true, (event) => {
-    info(`Loaded ${event.target.location}`);
-    return event.target.location && event.target.location.href.endsWith("popup.html");
-  });
-
-  let panelWindow = panelDocument.defaultView;
-  let panelBody = panelDocument.body;
+  let browser = yield openPanel(extension);
+  let panelWindow = browser.contentWindow;
+  let panelBody = panelWindow.document.body;
 
   function checkSize(expected) {
     is(panelWindow.innerHeight, expected, `Panel window should be ${expected}px tall`);
     is(panelBody.clientHeight, panelBody.scrollHeight,
       "Panel body should be tall enough to fit its contents");
 
     // Tolerate if it is 1px too wide, as that may happen with the current resizing method.
     ok(Math.abs(panelWindow.innerWidth - expected) <= 1, `Panel window should be ${expected}px wide`);
@@ -47,15 +67,228 @@ add_task(function* testPageActionPopupRe
   let sizes = [
     200,
     400,
     300,
   ];
 
   for (let size of sizes) {
     setSize(size);
-    yield BrowserTestUtils.waitForEvent(panelWindow, "resize");
+    yield awaitResize(browser);
     checkSize(size);
   }
 
   yield closeBrowserAction(extension);
   yield extension.unload();
 });
+
+function* testPopupSize(standardsMode, browserWin = window, arrowSide = "top") {
+  let docType = standardsMode ? "<!DOCTYPE html>" : "";
+
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      "browser_action": {
+        "default_popup": "popup.html",
+        "browser_style": false,
+      },
+    },
+
+    files: {
+      "popup.html": `${docType}
+        <html>
+          <head>
+            <meta charset="utf-8">
+            <style type="text/css">
+              body > span {
+                display: inline-block;
+                width: 10px;
+                height: 150px;
+                border: 2px solid black;
+              }
+              .big > span {
+                width: 300px;
+                height: 100px;
+              }
+              .bigger > span {
+                width: 150px;
+                height: 150px;
+              }
+              .huge > span {
+                height: ${2 * screen.height}px;
+              }
+            </style>
+          </head>
+          <body>
+            <span></span>
+            <span></span>
+            <span></span>
+            <span></span>
+          </body>
+        </html>`,
+    },
+  });
+
+  yield extension.startup();
+
+  if (arrowSide == "top") {
+    // Test the standalone panel for a toolbar button.
+    let browser = yield openPanel(extension, browserWin);
+    let win = browser.contentWindow;
+    let body = win.document.body;
+
+    let isStandards = win.document.compatMode != "BackCompat";
+    is(isStandards, standardsMode, "Document has the expected compat mode");
+
+    let {innerWidth, innerHeight} = win;
+
+    body.classList.add("bigger");
+    yield awaitResize(browser);
+
+    is(win.innerHeight, innerHeight, "Window height should not change");
+    ok(win.innerWidth > innerWidth, `Window width should increase (${win.innerWidth} > ${innerWidth})`);
+
+
+    body.classList.remove("bigger");
+    yield awaitResize(browser);
+
+    is(win.innerHeight, innerHeight, "Window height should not change");
+
+    // The getContentSize calculation is not always reliable to single-pixel
+    // precision.
+    ok(Math.abs(win.innerWidth - innerWidth) <= 1,
+       `Window width should return to approximately its original value (${win.innerWidth} ~= ${innerWidth})`);
+
+    yield closeBrowserAction(extension, browserWin);
+  }
+
+
+  // Test the PanelUI panel for a menu panel button.
+  let widget = getBrowserActionWidget(extension);
+  CustomizableUI.addWidgetToArea(widget.id, CustomizableUI.AREA_PANEL);
+
+  let browser = yield openPanel(extension, browserWin);
+  let win = browser.contentWindow;
+  let body = win.document.body;
+
+  let {panel} = browserWin.PanelUI;
+  let origPanelRect = panel.getBoundingClientRect();
+
+  // Check that the panel is still positioned as expected.
+  let checkPanelPosition = () => {
+    is(panel.getAttribute("side"), arrowSide, "Panel arrow is positioned as expected");
+
+    let panelRect = panel.getBoundingClientRect();
+    if (arrowSide == "top") {
+      ok(panelRect.top, origPanelRect.top, "Panel has not moved downwards");
+      ok(panelRect.bottom >= origPanelRect.bottom, `Panel has not shrunk from original size (${panelRect.bottom} >= ${origPanelRect.bottom})`);
+
+      let panelBottom = browserWin.mozInnerScreenY + panelRect.bottom;
+      ok(panelBottom <= screen.height, `Bottom of popup should be on-screen. (${panelBottom} <= ${screen.height})`);
+    } else {
+      ok(panelRect.bottom, origPanelRect.bottom, "Panel has not moved upwards");
+      ok(panelRect.top <= origPanelRect.top, `Panel has not shrunk from original size (${panelRect.top} <= ${origPanelRect.top})`);
+
+      let panelTop = browserWin.mozInnerScreenY + panelRect.top;
+      ok(panelTop >= 0, `Top of popup should be on-screen. (${panelTop} >= 0)`);
+    }
+  };
+
+
+  let isStandards = win.document.compatMode != "BackCompat";
+  is(isStandards, standardsMode, "Document has the expected compat mode");
+
+  // Wait long enough to make sure the initial resize debouncing timer has
+  // expired.
+  yield new Promise(resolve => setTimeout(resolve, 100));
+
+  // If the browser's preferred height is smaller than the initial height of the
+  // panel, then it will still take up the full available vertical space. Even
+  // so, we need to check that we've gotten the preferred height calculation
+  // correct, so check that explicitly.
+  let getHeight = () => parseFloat(browser.style.height);
+
+  let {innerWidth, innerHeight} = win;
+  let height = getHeight();
+
+
+  info("Increase body children's width. " +
+       "Expect them to wrap, and the frame to grow vertically rather than widen.");
+  body.className = "big";
+  yield awaitResize(browser);
+
+  ok(getHeight() > height, `Browser height should increase (${getHeight()} > ${height})`);
+
+  is(win.innerWidth, innerWidth, "Window width should not change");
+  ok(win.innerHeight >= innerHeight, `Window height should increase (${win.innerHeight} >= ${innerHeight})`);
+  is(win.scrollMaxY, 0, "Document should not be vertically scrollable");
+
+  checkPanelPosition();
+
+
+  info("Increase body children's width and height. " +
+       "Expect them to wrap, and the frame to grow vertically rather than widen.");
+  body.className = "bigger";
+  yield awaitResize(browser);
+
+  ok(getHeight() > height, `Browser height should increase (${getHeight()} > ${height})`);
+
+  is(win.innerWidth, innerWidth, "Window width should not change");
+  ok(win.innerHeight >= innerHeight, `Window height should increase (${win.innerHeight} >= ${innerHeight})`);
+  is(win.scrollMaxY, 0, "Document should not be vertically scrollable");
+
+  checkPanelPosition();
+
+
+  info("Increase body height beyond the height of the screen. " +
+       "Expect the panel to grow to accommodate, but not larger than the height of the screen.");
+  body.className = "huge";
+  yield awaitResize(browser);
+
+  ok(getHeight() > height, `Browser height should increase (${getHeight()} > ${height})`);
+
+  is(win.innerWidth, innerWidth, "Window width should not change");
+  ok(win.innerHeight > innerHeight, `Window height should increase (${win.innerHeight} > ${innerHeight})`);
+  ok(win.innerHeight < screen.height, `Window height be less than the screen height (${win.innerHeight} < ${screen.height})`);
+  ok(win.scrollMaxY > 0, `Document should be vertically scrollable (${win.scrollMaxY} > 0)`);
+
+  checkPanelPosition();
+
+
+  info("Restore original styling. Expect original dimensions.");
+  body.className = "";
+  yield awaitResize(browser);
+
+  is(getHeight(), height, "Browser height should return to its original value");
+
+  is(win.innerWidth, innerWidth, "Window width should not change");
+  is(win.innerHeight, innerHeight, "Window height should return to its original value");
+  is(win.scrollMaxY, 0, "Document should not be vertically scrollable");
+
+  checkPanelPosition();
+
+  yield closeBrowserAction(extension, browserWin);
+
+  yield extension.unload();
+}
+
+add_task(function* testBrowserActionMenuResizeStandards() {
+  yield testPopupSize(true);
+});
+
+add_task(function* testBrowserActionMenuResizeQuirks() {
+  yield testPopupSize(false);
+});
+
+// Test that we still make reasonable maximum size calculations when the window
+// is close enough to the bottom of the screen that the menu panel opens above,
+// rather than below, its button.
+add_task(function* testBrowserActionMenuResizeBottomArrow() {
+  const WIDTH = 800;
+  const HEIGHT = 400;
+
+  let win = yield BrowserTestUtils.openNewBrowserWindow();
+  win.resizeTo(WIDTH, HEIGHT);
+  win.moveTo(screen.width - WIDTH, screen.height - HEIGHT);
+
+  yield testPopupSize(false, win, "bottom");
+
+  yield BrowserTestUtils.closeWindow(win);
+});
--- a/browser/themes/shared/customizableui/panelUI.inc.css
+++ b/browser/themes/shared/customizableui/panelUI.inc.css
@@ -272,16 +272,20 @@ panelmultiview[nosubviews=true] > .panel
 .cui-widget-panel[viewId^=PanelUI-webext-] > .panel-arrowcontainer > .panel-arrowcontent {
   padding: 0;
 }
 
 .cui-widget-panelview[id^=PanelUI-webext-] {
   border-radius: 3.5px;
 }
 
+panelview[id^=PanelUI-webext-] {
+  overflow: hidden;
+}
+
 panelview:not([mainview]) .toolbarbutton-text,
 .cui-widget-panel toolbarbutton > .toolbarbutton-text {
   text-align: start;
   display: -moz-box;
 }
 
 .cui-widget-panel > .panel-arrowcontainer > .panel-arrowcontent {
   padding: 4px 0;