Bug 1461522 - Allow { height: "auto" } in HTMLTooltip setContent and make it the default; r?jdescottes draft
authorBrian Birtles <birtles@gmail.com>
Thu, 28 Jun 2018 15:03:57 +0900
changeset 813897 14ca57e648d6d647dfdaf2f9857e97ba5bd90aed
parent 813896 01dce05cc1af0e1706c0bf7e1ec79af1dad68b37
child 813898 9544afc6116630a97af5f79cd91ee5756bf3b727
push id115042
push userbbirtles@mozilla.com
push dateWed, 04 Jul 2018 04:36:27 +0000
reviewersjdescottes
bugs1461522
milestone63.0a1
Bug 1461522 - Allow { height: "auto" } in HTMLTooltip setContent and make it the default; r?jdescottes The current default value for height of Infinity has the unfortunate side effect that, when combined with using a XUL wrapper, there will be a large filler element stretching vertically on one side of the tooltip that effectively neuters all content beneath it. While this is probably fine for tooltips that are shown on hover, it is problematic if we want to use this for DevTools menus because it means the user is unable to click anything above/below the menu so long as it is open (which can be particularly problematic once we make HTMLTooltip support the "Disable popup autohide" feature"). Even if we were to decide that clicks outside the tooltip should be consumed anyway we would still have the problem that hover styles don't apply in this "dead" region. As a result, this patch makes the { height: Infinity } behaviour opt-in for those tooltips that really need it. For most uses, however, a height calculated when the tooltip is shown should be sufficient (and later in this patch series we will add a mechanism to HTMLTooltip to explicitly request it recalculate its size and position in response to content changes). This patch introduces the { height: "auto" } mechanism and also reverses the order in which size/position properties are calculated to match the regular manner in which layout is performed: widths first, then heights. MozReview-Commit-ID: 7BeVkxGVhYn
devtools/client/shared/test/browser.ini
devtools/client/shared/test/browser_html_tooltip_height-auto.js
devtools/client/shared/widgets/tooltip/EventTooltipHelper.js
devtools/client/shared/widgets/tooltip/HTMLTooltip.js
--- a/devtools/client/shared/test/browser.ini
+++ b/devtools/client/shared/test/browser.ini
@@ -133,16 +133,17 @@ skip-if = e10s # Bug 1221911, bug 122228
 [browser_html_tooltip-01.js]
 [browser_html_tooltip-02.js]
 [browser_html_tooltip-03.js]
 [browser_html_tooltip-04.js]
 [browser_html_tooltip-05.js]
 [browser_html_tooltip_arrow-01.js]
 [browser_html_tooltip_arrow-02.js]
 [browser_html_tooltip_consecutive-show.js]
+[browser_html_tooltip_height-auto.js]
 [browser_html_tooltip_hover.js]
 [browser_html_tooltip_offset.js]
 [browser_html_tooltip_rtl.js]
 [browser_html_tooltip_variable-height.js]
 [browser_html_tooltip_width-auto.js]
 [browser_html_tooltip_xul-wrapper.js]
 [browser_html_tooltip_zoom.js]
 [browser_inplace-editor-01.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/shared/test/browser_html_tooltip_height-auto.js
@@ -0,0 +1,67 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+/* import-globals-from helper_html_tooltip.js */
+
+"use strict";
+
+/**
+ * Test the HTMLTooltip content can automatically calculate its height based on
+ * content.
+ */
+
+const HTML_NS = "http://www.w3.org/1999/xhtml";
+const TEST_URI = CHROME_URL_ROOT + "doc_html_tooltip.xul";
+
+const {HTMLTooltip} =
+  require("devtools/client/shared/widgets/tooltip/HTMLTooltip");
+loadHelperScript("helper_html_tooltip.js");
+
+let useXulWrapper;
+
+add_task(async function() {
+  await addTab("about:blank");
+  const [,, doc] = await createHost("bottom", TEST_URI);
+
+  info("Run tests for a Tooltip without using a XUL panel");
+  useXulWrapper = false;
+  await runTests(doc);
+
+  info("Run tests for a Tooltip with a XUL panel");
+  useXulWrapper = true;
+  await runTests(doc);
+});
+
+async function runTests(doc) {
+  const tooltip = new HTMLTooltip(doc, {useXulWrapper});
+  info("Create tooltip content height to 150px");
+  const tooltipContent = doc.createElementNS(HTML_NS, "div");
+  tooltipContent.style.cssText =
+    "width: 300px; height: 150px; background: red;";
+
+  info("Set tooltip content using width:auto and height:auto");
+  tooltip.setContent(tooltipContent);
+
+  info("Show the tooltip and check the tooltip panel dimensions.");
+  await showTooltip(tooltip, doc.getElementById("box1"));
+
+  let panelRect = tooltip.panel.getBoundingClientRect();
+  is(panelRect.width, 300, "Tooltip panel has the expected width.");
+  is(panelRect.height, 150, "Tooltip panel has the expected width.");
+
+  await hideTooltip(tooltip);
+
+  info("Set tooltip content using fixed width and height:auto");
+  tooltipContent.style.cssText =
+    "width: auto; height: 200px; background: red;";
+  tooltip.setContent(tooltipContent, { width: 400 });
+
+  info("Show the tooltip and check the tooltip panel height.");
+  await showTooltip(tooltip, doc.getElementById("box1"));
+
+  panelRect = tooltip.panel.getBoundingClientRect();
+  is(panelRect.height, 200, "Tooltip panel has the expected width.");
+
+  await hideTooltip(tooltip);
+
+  tooltip.destroy();
+}
--- a/devtools/client/shared/widgets/tooltip/EventTooltipHelper.js
+++ b/devtools/client/shared/widgets/tooltip/EventTooltipHelper.js
@@ -192,17 +192,20 @@ EventTooltip.prototype = {
       });
 
       content.className = "event-tooltip-content-box";
       this.container.appendChild(content);
 
       this._addContentListeners(header);
     }
 
-    this._tooltip.setContent(this.container, {width: CONTAINER_WIDTH});
+    this._tooltip.setContent(
+      this.container,
+      {width: CONTAINER_WIDTH, height: Infinity}
+    );
     this._tooltip.on("hidden", this.destroy);
   },
 
   _addContentListeners: function(header) {
     header.addEventListener("click", this._headerClicked);
   },
 
   _headerClicked: function(event) {
--- a/devtools/client/shared/widgets/tooltip/HTMLTooltip.js
+++ b/devtools/client/shared/widgets/tooltip/HTMLTooltip.js
@@ -250,16 +250,18 @@ function HTMLTooltip(toolboxDoc, {
   } = {}) {
   EventEmitter.decorate(this);
 
   this.doc = toolboxDoc;
   this.type = type;
   this.autofocus = autofocus;
   this.consumeOutsideClicks = consumeOutsideClicks;
   this.useXulWrapper = this._isXUL() && useXulWrapper;
+  this.preferredWidth = "auto";
+  this.preferredHeight = "auto";
 
   // The top window is used to attach click event listeners to close the tooltip if the
   // user clicks on the content page.
   this.topWindow = this._getTopWindow();
 
   this._position = null;
 
   this._onClick = this._onClick.bind(this);
@@ -321,21 +323,31 @@ HTMLTooltip.prototype = {
    * Set the tooltip content element. The preferred width/height should also be
    * specified here.
    *
    * @param {Element} content
    *        The tooltip content, should be a HTML element.
    * @param {Object}
    *        - {Number} width: preferred width for the tooltip container. If not specified
    *          the tooltip container will be measured before being displayed, and the
-   *          measured width will be used as preferred width.
-   *        - {Number} height: optional, preferred height for the tooltip container. If
-   *          not specified, the tooltip will be able to use all the height available.
+   *          measured width will be used as the preferred width.
+   *        - {Number} height: preferred height for the tooltip container. If
+   *          not specified the tooltip container will be measured before being
+   *          displayed, and the measured height will be used as the preferred
+   *          height.
+   *
+   *          For tooltips whose content height may change while being
+   *          displayed, the special value Infinity may be used to produce
+   *          a flexible container that accommodates resizing content. Note,
+   *          however, that when used in combination with the XUL wrapper the
+   *          unfilled part of this container will consume all mouse events
+   *          making content behind this area inaccessible until the tooltip is
+   *          dismissed.
    */
-  setContent: function(content, {width = "auto", height = Infinity} = {}) {
+  setContent: function(content, {width = "auto", height = "auto"} = {}) {
     this.preferredWidth = width;
     this.preferredHeight = height;
 
     this.panel.innerHTML = "";
     this.panel.appendChild(content);
   },
 
   /**
@@ -361,54 +373,82 @@ HTMLTooltip.prototype = {
     let anchorRect = getRelativeRect(anchor, this.doc);
     if (this.useXulWrapper) {
       anchorRect = this._convertToScreenRect(anchorRect);
     }
 
     // Get viewport size
     const viewportRect = this._getViewportRect();
 
-    const themeHeight = EXTRA_HEIGHT[this.type] + 2 * EXTRA_BORDER[this.type];
-    const preferredHeight = this.preferredHeight + themeHeight;
+    // Calculate the horizonal position and width
+    let preferredWidth;
+    // Record the height too since it might save us from having to look it up
+    // later.
+    let measuredHeight;
+    if (this.preferredWidth === "auto") {
+      // Reset any styles that constrain the dimensions we want to calculate.
+      this.container.style.width = "auto";
+      if (this.preferredHeight === "auto") {
+        this.container.style.height = "auto";
+      }
+      ({
+        width: preferredWidth,
+        height: measuredHeight,
+      } = this._measureContainerSize());
+    } else {
+      const themeWidth = 2 * EXTRA_BORDER[this.type];
+      preferredWidth = this.preferredWidth + themeWidth;
+    }
+
+    const anchorWin = anchor.ownerDocument.defaultView;
+    const isRtl = anchorWin.getComputedStyle(anchor).direction === "rtl";
+    const {left, width, arrowLeft} = calculateHorizontalPosition(
+      anchorRect, viewportRect, preferredWidth, this.type, x, isRtl);
+
+    // If we constrained the width, then any measured height we have is no
+    // longer valid.
+    if (measuredHeight && width !== preferredWidth) {
+      measuredHeight = undefined;
+    }
+
+    // Apply width and arrow positioning
+    this.container.style.width = width + "px";
+    if (this.type === TYPE.ARROW) {
+      this.arrow.style.left = arrowLeft + "px";
+    }
+
+    // Calculate the vertical position and height
+    let preferredHeight;
+    if (this.preferredHeight === "auto") {
+      if (measuredHeight) {
+        this.container.style.height = "auto";
+        preferredHeight = measuredHeight;
+      } else {
+        ({ height: preferredHeight } = this._measureContainerSize());
+      }
+    } else {
+      const themeHeight = EXTRA_HEIGHT[this.type] + 2 * EXTRA_BORDER[this.type];
+      preferredHeight = this.preferredHeight + themeHeight;
+    }
 
     const {top, height, computedPosition} =
       calculateVerticalPosition(anchorRect, viewportRect, preferredHeight, position, y);
 
     this._position = computedPosition;
-    // Apply height before measuring the content width (if width="auto").
     const isTop = computedPosition === POSITION.TOP;
     this.container.classList.toggle("tooltip-top", isTop);
     this.container.classList.toggle("tooltip-bottom", !isTop);
 
     // If the preferred height is set to Infinity, the tooltip container should grow based
     // on its content's height and use as much height as possible.
     this.container.classList.toggle("tooltip-flexible-height",
       this.preferredHeight === Infinity);
 
     this.container.style.height = height + "px";
 
-    let preferredWidth;
-    if (this.preferredWidth === "auto") {
-      preferredWidth = this._measureContainerWidth();
-    } else {
-      const themeWidth = 2 * EXTRA_BORDER[this.type];
-      preferredWidth = this.preferredWidth + themeWidth;
-    }
-
-    const anchorWin = anchor.ownerDocument.defaultView;
-    const isRtl = anchorWin.getComputedStyle(anchor).direction === "rtl";
-    const {left, width, arrowLeft} = calculateHorizontalPosition(
-      anchorRect, viewportRect, preferredWidth, this.type, x, isRtl);
-
-    this.container.style.width = width + "px";
-
-    if (this.type === TYPE.ARROW) {
-      this.arrow.style.left = arrowLeft + "px";
-    }
-
     if (this.useXulWrapper) {
       await this._showXulWrapperAt(left, top);
     } else {
       this.container.style.left = left + "px";
       this.container.style.top = top + "px";
     }
 
     this.container.classList.add("tooltip-visible");
@@ -450,33 +490,37 @@ HTMLTooltip.prototype = {
         width: availWidth,
         height: availHeight,
       };
     }
 
     return this.doc.documentElement.getBoundingClientRect();
   },
 
-  _measureContainerWidth: function() {
+  _measureContainerSize: function() {
     const xulParent = this.container.parentNode;
     if (this.useXulWrapper && !this.isVisible()) {
       // Move the container out of the XUL Panel to measure it.
       this.doc.documentElement.appendChild(this.container);
     }
 
     this.container.classList.add("tooltip-hidden");
-    this.container.style.width = "auto";
-    const width = this.container.getBoundingClientRect().width;
+    // Set either of the tooltip-top or tooltip-bottom styles so that we get an
+    // accurate height. We're assuming that the two styles will be symmetrical
+    // and that we will clear this as necessary later.
+    this.container.classList.add("tooltip-top");
+    this.container.classList.remove("tooltip-bottom");
+    const { width, height } = this.container.getBoundingClientRect();
     this.container.classList.remove("tooltip-hidden");
 
     if (this.useXulWrapper && !this.isVisible()) {
       xulParent.appendChild(this.container);
     }
 
-    return width;
+    return { width, height };
   },
 
   /**
    * Hide the current tooltip. The event "hidden" will be fired when the tooltip
    * is hidden.
    */
   async hide() {
     this.doc.defaultView.clearTimeout(this.attachEventsTimer);