Bug 1266450 - part2: remove iframe container for HTML tooltip;r=bgrins draft
authorJulian Descottes <jdescottes@mozilla.com>
Mon, 30 May 2016 23:02:58 +0200
changeset 376618 4470f401840d3aea01eb7c120c989e3553d24f99
parent 376617 f57c6988c9091165de91367260cf314f954362cc
child 376619 32c9f5c6462180168a0ffeb26f047ed516ba6932
push id20630
push userjdescottes@mozilla.com
push dateWed, 08 Jun 2016 11:49:53 +0000
reviewersbgrins
bugs1266450
milestone50.0a1
Bug 1266450 - part2: remove iframe container for HTML tooltip;r=bgrins In order to have tooltips with a variable height, the tooltip container should be allowed to resize itself on the fly, which cannot be achieved with an iframe. This changeset makes the HTMLTooltip rely on a HTML container inserted in the XUL document directly. This allows to go back to a synchronous API which also simplifies the implementation. MozReview-Commit-ID: EDcsnVSKmeU
devtools/client/inspector/markup/markup.js
devtools/client/jar.mn
devtools/client/shared/test/browser_html_tooltip-03.js
devtools/client/shared/test/browser_html_tooltip_arrow-01.js
devtools/client/shared/test/browser_html_tooltip_arrow-02.js
devtools/client/shared/widgets/HTMLTooltip.js
devtools/client/shared/widgets/tooltip-frame.xhtml
devtools/client/shared/widgets/tooltip/ImageTooltipHelper.js
--- a/devtools/client/inspector/markup/markup.js
+++ b/devtools/client/inspector/markup/markup.js
@@ -2699,20 +2699,20 @@ MarkupElementContainer.prototype = Herit
       let { data, size } = yield this._getPreview();
       // The preview is ready.
       let options = {
         naturalWidth: size.naturalWidth,
         naturalHeight: size.naturalHeight,
         maxDim: Services.prefs.getIntPref(PREVIEW_MAX_DIM_PREF)
       };
 
-      yield setImageTooltip(tooltip, this.markup.doc, data, options);
+      setImageTooltip(tooltip, this.markup.doc, data, options);
     } catch (e) {
       // Indicate the failure but show the tooltip anyway.
-      yield setBrokenImageTooltip(tooltip, this.markup.doc);
+      setBrokenImageTooltip(tooltip, this.markup.doc);
     }
     return true;
   }),
 
   copyImageDataUri: function () {
     // We need to send again a request to gettooltipData even if one was sent
     // for the tooltip, because we want the full-size image
     this.node.getImageData().then(data => {
--- a/devtools/client/jar.mn
+++ b/devtools/client/jar.mn
@@ -123,17 +123,16 @@ devtools.jar:
     content/inspector/inspector.xul (inspector/inspector.xul)
     content/inspector/inspector.css (inspector/inspector.css)
     content/framework/connect/connect.xhtml (framework/connect/connect.xhtml)
     content/framework/connect/connect.css (framework/connect/connect.css)
     content/framework/connect/connect.js (framework/connect/connect.js)
     content/shared/widgets/graphs-frame.xhtml (shared/widgets/graphs-frame.xhtml)
     content/shared/widgets/spectrum-frame.xhtml (shared/widgets/spectrum-frame.xhtml)
     content/shared/widgets/cubic-bezier-frame.xhtml (shared/widgets/cubic-bezier-frame.xhtml)
-    content/shared/widgets/tooltip-frame.xhtml (shared/widgets/tooltip-frame.xhtml)
     content/shared/widgets/cubic-bezier.css (shared/widgets/cubic-bezier.css)
     content/shared/widgets/mdn-docs-frame.xhtml (shared/widgets/mdn-docs-frame.xhtml)
     content/shared/widgets/mdn-docs.css (shared/widgets/mdn-docs.css)
     content/shared/widgets/filter-frame.xhtml (shared/widgets/filter-frame.xhtml)
     content/shared/widgets/filter-widget.css (shared/widgets/filter-widget.css)
     content/eyedropper/eyedropper.xul (eyedropper/eyedropper.xul)
     content/eyedropper/crosshairs.css (eyedropper/crosshairs.css)
     content/eyedropper/nocursor.css (eyedropper/nocursor.css)
--- a/devtools/client/shared/test/browser_html_tooltip-03.js
+++ b/devtools/client/shared/test/browser_html_tooltip-03.js
@@ -10,33 +10,35 @@
 
 const HTML_NS = "http://www.w3.org/1999/xhtml";
 const TEST_URI = `data:text/xml;charset=UTF-8,<?xml version="1.0"?>
   <?xml-stylesheet href="chrome://global/skin/global.css"?>
   <?xml-stylesheet href="chrome://devtools/skin/tooltips.css"?>
   <window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
    title="Tooltip test">
     <vbox flex="1">
-      <hbox id="box1" flex="1">test1</hbox>
+      <hbox id="box1" flex="1">
+        <textbox></textbox>
+      </hbox>
       <hbox id="box2" flex="1">test2</hbox>
       <hbox id="box3" flex="1">
         <textbox id="box3-input"></textbox>
       </hbox>
       <hbox id="box4" flex="1">
         <textbox id="box4-input"></textbox>
       </hbox>
     </vbox>
   </window>`;
 
 const {HTMLTooltip} = require("devtools/client/shared/widgets/HTMLTooltip");
 loadHelperScript("helper_html_tooltip.js");
 
 add_task(function* () {
   yield addTab("about:blank");
-  let [,, doc] = yield createHost("bottom", TEST_URI);
+  let [, , doc] = yield createHost("bottom", TEST_URI);
 
   yield testNoAutoFocus(doc);
   yield testAutoFocus(doc);
   yield testAutoFocusPreservesFocusChange(doc);
 });
 
 function* testNoAutoFocus(doc) {
   yield focusNode(doc, "#box4-input");
--- a/devtools/client/shared/test/browser_html_tooltip_arrow-01.js
+++ b/devtools/client/shared/test/browser_html_tooltip_arrow-01.js
@@ -67,28 +67,28 @@ add_task(function* () {
   let elements = [...doc.querySelectorAll(".anchor")];
   for (let el of elements) {
     info("Display the tooltip on an anchor.");
     yield showTooltip(tooltip, el);
 
     let arrow = tooltip.arrow;
     ok(arrow, "Tooltip has an arrow");
 
-    // Get the geometry of the anchor, the tooltip frame & arrow.
+    // Get the geometry of the anchor, the tooltip panel & arrow.
     let arrowBounds = arrow.getBoxQuads({relativeTo: doc})[0].bounds;
-    let frameBounds = tooltip.frame.getBoxQuads({relativeTo: doc})[0].bounds;
+    let panelBounds = tooltip.panel.getBoxQuads({relativeTo: doc})[0].bounds;
     let anchorBounds = el.getBoxQuads({relativeTo: doc})[0].bounds;
 
     let intersects = arrowBounds.left <= anchorBounds.right &&
                      arrowBounds.right >= anchorBounds.left;
     let isBlockedByViewport = arrowBounds.left == 0 ||
                               arrowBounds.right == docRight;
     ok(intersects || isBlockedByViewport,
       "Tooltip arrow is aligned with the anchor, or stuck on viewport's edge.");
 
-    let isInFrame = arrowBounds.left >= frameBounds.left &&
-                    arrowBounds.right <= frameBounds.right;
-    ok(isInFrame,
-      "The tooltip arrow remains inside the tooltip frame horizontally");
+    let isInPanel = arrowBounds.left >= panelBounds.left &&
+                    arrowBounds.right <= panelBounds.right;
+    ok(isInPanel,
+      "The tooltip arrow remains inside the tooltip panel horizontally");
 
     yield hideTooltip(tooltip);
   }
 });
--- a/devtools/client/shared/test/browser_html_tooltip_arrow-02.js
+++ b/devtools/client/shared/test/browser_html_tooltip_arrow-02.js
@@ -61,27 +61,27 @@ add_task(function* () {
   let elements = [...doc.querySelectorAll(".anchor")];
   for (let el of elements) {
     info("Display the tooltip on an anchor.");
     yield showTooltip(tooltip, el);
 
     let arrow = tooltip.arrow;
     ok(arrow, "Tooltip has an arrow");
 
-    // Get the geometry of the anchor, the tooltip frame & arrow.
+    // Get the geometry of the anchor, the tooltip panel & arrow.
     let arrowBounds = arrow.getBoxQuads({relativeTo: doc})[0].bounds;
-    let frameBounds = tooltip.frame.getBoxQuads({relativeTo: doc})[0].bounds;
+    let panelBounds = tooltip.panel.getBoxQuads({relativeTo: doc})[0].bounds;
     let anchorBounds = el.getBoxQuads({relativeTo: doc})[0].bounds;
 
     let intersects = arrowBounds.left <= anchorBounds.right &&
                      arrowBounds.right >= anchorBounds.left;
     let isBlockedByViewport = arrowBounds.left == 0 ||
                               arrowBounds.right == docRight;
     ok(intersects || isBlockedByViewport,
       "Tooltip arrow is aligned with the anchor, or stuck on viewport's edge.");
 
-    let isInFrame = arrowBounds.left >= frameBounds.left &&
-                    arrowBounds.right <= frameBounds.right;
-    ok(isInFrame,
-      "The tooltip arrow remains inside the tooltip frame horizontally");
+    let isInPanel = arrowBounds.left >= panelBounds.left &&
+                    arrowBounds.right <= panelBounds.right;
+    ok(isInPanel,
+      "The tooltip arrow remains inside the tooltip panel horizontally");
     yield hideTooltip(tooltip);
   }
 });
--- a/devtools/client/shared/widgets/HTMLTooltip.js
+++ b/devtools/client/shared/widgets/HTMLTooltip.js
@@ -7,19 +7,16 @@
 "use strict";
 
 const EventEmitter = require("devtools/shared/event-emitter");
 const {TooltipToggle} = require("devtools/client/shared/widgets/tooltip/TooltipToggle");
 
 const XUL_NS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
 const XHTML_NS = "http://www.w3.org/1999/xhtml";
 
-const IFRAME_URL = "chrome://devtools/content/shared/widgets/tooltip-frame.xhtml";
-const IFRAME_CONTAINER_ID = "tooltip-iframe-container";
-
 const POSITION = {
   TOP: "top",
   BOTTOM: "bottom",
 };
 
 module.exports.POSITION = POSITION;
 
 const TYPE = {
@@ -74,61 +71,33 @@ function HTMLTooltip(toolbox,
   this._onClick = this._onClick.bind(this);
 
   this._toggle = new TooltipToggle(this);
   this.startTogglingOnHover = this._toggle.start.bind(this._toggle);
   this.stopTogglingOnHover = this._toggle.stop.bind(this._toggle);
 
   this.container = this._createContainer();
 
-  // Promise that will resolve when the container can be filled with content.
-  this.containerReady = new Promise(resolve => {
-    if (this._isXUL()) {
-      // In XUL context, load a placeholder document in the iframe container.
-      let onLoad = () => {
-        this.frame.removeEventListener("load", onLoad, true);
-        resolve();
-      };
-
-      this.frame.addEventListener("load", onLoad, true);
-      this.frame.setAttribute("src", IFRAME_URL);
-      this.doc.querySelector("window").appendChild(this.container);
-    } else {
-      // In non-XUL context the container is ready to use as is.
-      this.doc.body.appendChild(this.container);
-      resolve();
-    }
-  });
+  if (this._isXUL()) {
+    this.doc.querySelector("window").appendChild(this.container);
+  } else {
+    // In non-XUL context the container is ready to use as is.
+    this.doc.body.appendChild(this.container);
+  }
 }
 
 module.exports.HTMLTooltip = HTMLTooltip;
 
 HTMLTooltip.prototype = {
   /**
-   * The tooltip frame is the child of the tooltip container that will only
-   * contain the tooltip content (and not the arrow or any other tooltip styling
-   * element).
-   * In XUL contexts, this is an iframe. In non XUL contexts this is a div,
-   * which also happens to be the tooltip.panel property.
-   */
-  get frame() {
-    return this.container.querySelector(".tooltip-panel");
-  },
-
-  /**
    * The tooltip panel is the parentNode of the tooltip content provided in
    * setContent().
    */
   get panel() {
-    if (!this._isXUL()) {
-      return this.frame;
-    }
-    // In XUL context, the content is wrapped in an iframe.
-    let win = this.frame.contentWindow.wrappedJSObject;
-    return win.document.getElementById(IFRAME_CONTAINER_ID);
+    return this.container.querySelector(".tooltip-panel");
   },
 
   /**
    * The arrow element. Might be null depending on the tooltip type.
    */
   get arrow() {
     return this.container.querySelector(".tooltip-arrow");
   },
@@ -148,62 +117,58 @@ HTMLTooltip.prototype = {
    */
   setContent: function (content, width, height) {
     let themeHeight = EXTRA_HEIGHT[this.type] + 2 * EXTRA_BORDER[this.type];
     let themeWidth = 2 * EXTRA_BORDER[this.type];
 
     this.preferredWidth = width + themeWidth;
     this.preferredHeight = height + themeHeight;
 
-    return this.containerReady.then(() => {
-      this.panel.innerHTML = "";
-      this.panel.appendChild(content);
-    });
+    this.panel.innerHTML = "";
+    this.panel.appendChild(content);
   },
 
   /**
    * Show the tooltip next to the provided anchor element. A preferred position
    * can be set. The event "shown" will be fired after the tooltip is displayed.
    *
    * @param {Element} anchor
    *        The reference element with which the tooltip should be aligned
    * @param {Object}
    *        - {String} position: optional, possible values: top|bottom
    *          If layout permits, the tooltip will be displayed on top/bottom
    *          of the anchor. If ommitted, the tooltip will be displayed where
    *          more space is available.
    */
   show: function (anchor, {position} = {}) {
-    this.containerReady.then(() => {
-      let computedPosition = this._findBestPosition(anchor, position);
+    let computedPosition = this._findBestPosition(anchor, position);
 
-      let isTop = computedPosition.position === POSITION.TOP;
-      this.container.classList.toggle("tooltip-top", isTop);
-      this.container.classList.toggle("tooltip-bottom", !isTop);
+    let isTop = computedPosition.position === POSITION.TOP;
+    this.container.classList.toggle("tooltip-top", isTop);
+    this.container.classList.toggle("tooltip-bottom", !isTop);
 
-      this.container.style.width = computedPosition.width + "px";
-      this.container.style.height = computedPosition.height + "px";
-      this.container.style.top = computedPosition.top + "px";
-      this.container.style.left = computedPosition.left + "px";
+    this.container.style.width = computedPosition.width + "px";
+    this.container.style.height = computedPosition.height + "px";
+    this.container.style.top = computedPosition.top + "px";
+    this.container.style.left = computedPosition.left + "px";
 
-      if (this.type === TYPE.ARROW) {
-        this.arrow.style.left = computedPosition.arrowLeft + "px";
-      }
+    if (this.type === TYPE.ARROW) {
+      this.arrow.style.left = computedPosition.arrowLeft + "px";
+    }
 
-      this.container.classList.add("tooltip-visible");
+    this.container.classList.add("tooltip-visible");
 
-      this.attachEventsTimer = this.doc.defaultView.setTimeout(() => {
-        this._focusedElement = this.doc.activeElement;
-        if (this.autofocus) {
-          this.frame.focus();
-        }
-        this.topWindow.addEventListener("click", this._onClick, true);
-        this.emit("shown");
-      }, 0);
-    });
+    this.attachEventsTimer = this.doc.defaultView.setTimeout(() => {
+	  this._focusedElement = this.doc.activeElement;
+      if (this.autofocus) {
+        this.panel.focus();
+      }
+      this.topWindow.addEventListener("click", this._onClick, true);
+      this.emit("shown");
+    }, 0);
   },
 
   /**
    * Hide the current tooltip. The event "hidden" will be fired when the tooltip
    * is hidden.
    */
   hide: function () {
     this.doc.defaultView.clearTimeout(this.attachEventsTimer);
@@ -237,22 +202,17 @@ HTMLTooltip.prototype = {
     this.container.remove();
   },
 
   _createContainer: function () {
     let container = this.doc.createElementNS(XHTML_NS, "div");
     container.setAttribute("type", this.type);
     container.classList.add("tooltip-container");
 
-    let html;
-    if (this._isXUL()) {
-      html = '<iframe class="devtools-tooltip-iframe tooltip-panel"></iframe>';
-    } else {
-      html = '<div class="tooltip-panel"></div>';
-    }
+    let html = '<div class="tooltip-panel"></div>';
 
     if (this.type === TYPE.ARROW) {
       html += '<div class="tooltip-arrow"></div>';
     }
     container.innerHTML = html;
     return container;
   },
 
deleted file mode 100644
--- a/devtools/client/shared/widgets/tooltip-frame.xhtml
+++ /dev/null
@@ -1,29 +0,0 @@
-<?xml version="1.0" encoding="UTF-8"?>
-<!-- This Source Code Form is subject to the terms of the Mozilla Public
-   - License, v. 2.0. If a copy of the MPL was not distributed with this
-   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
-<!DOCTYPE html>
-
-<html xmlns="http://www.w3.org/1999/xhtml">
-<head>
-  <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"/>
-  <script type="application/javascript;version=1.8" src="chrome://devtools/content/shared/theme-switching.js"/>
-  <style>
-    html, body, #tooltip-iframe-container {
-      margin: 0;
-      padding: 0;
-      width: 100%;
-      height: 100%;
-      overflow: hidden;
-      color: var(--theme-body-color);
-    }
-
-    :root[platform="linux"] body {
-      font-size: 80%;
-    }
-  </style>
-</head>
-<body role="application">
-  <div id="tooltip-iframe-container"></div>
-</body>
-</html>
--- a/devtools/client/shared/widgets/tooltip/ImageTooltipHelper.js
+++ b/devtools/client/shared/widgets/tooltip/ImageTooltipHelper.js
@@ -57,18 +57,16 @@ function getImageDimensions(doc, imageUr
  *        A document element to create the HTML elements needed for the tooltip
  * @param {String} imageUrl
  *        Absolute URL of the image to display in the tooltip
  * @param {Object} options
  *        - {Number} naturalWidth mandatory, width of the image to display
  *        - {Number} naturalHeight mandatory, height of the image to display
  *        - {Number} maxDim optional, max width/height of the preview
  *        - {Boolean} hideDimensionLabel optional, pass true to hide the label
- * @return {Promise} promise that will resolve when the tooltip content has been
- *         set
  */
 function setImageTooltip(tooltip, doc, imageUrl, options) {
   let {naturalWidth, naturalHeight, hideDimensionLabel, maxDim} = options;
   maxDim = maxDim || MAX_DIMENSION;
 
   let imgHeight = naturalHeight;
   let imgWidth = naturalWidth;
   if (imgHeight > maxDim || imgWidth > maxDim) {
@@ -108,38 +106,36 @@ function setImageTooltip(tooltip, doc, i
 
   // Calculate tooltip dimensions
   let height = imgHeight + 2 * IMAGE_PADDING;
   if (!hideDimensionLabel) {
     height += LABEL_HEIGHT;
   }
   let width = Math.max(CONTAINER_MIN_WIDTH, imgWidth + 2 * IMAGE_PADDING);
 
-  return tooltip.setContent(div, width, height);
+  tooltip.setContent(div, width, height);
 }
 
 /*
  * Set the tooltip content of a provided HTMLTooltip instance to display a
  * fallback error message when an image preview tooltip can not be displayed.
  *
  * @param {HTMLTooltip} tooltip
  *        The tooltip instance on which the image preview content should be set
  * @param {Document} doc
  *        A document element to create the HTML elements needed for the tooltip
- * @return {Promise} promise that will resolve when the tooltip content has been
- *         set
  */
 function setBrokenImageTooltip(tooltip, doc) {
   let div = doc.createElementNS(XHTML_NS, "div");
   div.style.cssText = `
     box-sizing: border-box;
     height: 100%;
     text-align: center;
     line-height: 30px;`;
 
   let message = GetStringFromName("previewTooltip.image.brokenImage");
   div.textContent = message;
-  return tooltip.setContent(div, 150, 30);
+  tooltip.setContent(div, 150, 30);
 }
 
 module.exports.getImageDimensions = getImageDimensions;
 module.exports.setImageTooltip = setImageTooltip;
 module.exports.setBrokenImageTooltip = setBrokenImageTooltip;