Bug 1590408 - Backed out changeset 84708a4f040d from Bug 1552146 r=miker
authorJulian Descottes <jdescottes@mozilla.com>
Tue, 12 Nov 2019 15:45:57 +0000
changeset 501662 40b9a55b447d71a80645db1f043f61047f018663
parent 501661 452480b6db9cbf3b3125274898337425f1286b8c
child 501663 f1293ec6b6da76ec757a420e2969f444d1531738
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmiker
bugs1590408, 1552146
milestone72.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 1590408 - Backed out changeset 84708a4f040d from Bug 1552146 r=miker The positioning of the arrow was correct, but due to the way the arrow was rotated, the visual center of the arrow did not match the center of the arrow box Backing out the JS change here, CSS patch later in the queue Differential Revision: https://phabricator.services.mozilla.com/D52668
devtools/client/shared/test/browser_html_tooltip_doorhanger-02.js
devtools/client/shared/test/browser_html_tooltip_offset.js
devtools/client/shared/test/browser_html_tooltip_rtl.js
devtools/client/shared/test/head.js
devtools/client/shared/test/helper_html_tooltip.js
devtools/client/shared/widgets/tooltip/HTMLTooltip.js
--- a/devtools/client/shared/test/browser_html_tooltip_doorhanger-02.js
+++ b/devtools/client/shared/test/browser_html_tooltip_doorhanger-02.js
@@ -59,17 +59,17 @@ async function runTests(doc) {
     const center = bounds => bounds.left + bounds.width / 2;
     const delta = Math.abs(center(anchorBounds) - center(arrowBounds));
     const describeBounds = bounds =>
       `${bounds.left}<--[${center(bounds)}]-->${bounds.right}`;
     const params =
       `anchor: ${describeBounds(anchorBounds)}, ` +
       `arrow: ${describeBounds(arrowBounds)}`;
     ok(
-      delta <= 1,
+      delta < 1,
       `Arrow center is roughly aligned with anchor center (${params})`
     );
 
     await hideTooltip(tooltip);
   }
 
   tooltip.destroy();
 }
--- a/devtools/client/shared/test/browser_html_tooltip_offset.js
+++ b/devtools/client/shared/test/browser_html_tooltip_offset.js
@@ -32,92 +32,66 @@ add_task(async function() {
 
   const div = doc.createElementNS(HTML_NS, "div");
   div.style.height = "100px";
   div.style.boxSizing = "border-box";
   div.textContent = "tooltip";
   tooltip.panel.appendChild(div);
   tooltip.setContentSize({ width: 50, height: 100 });
 
-  const { offsetTop, offsetLeft } = getOffsets(doc);
-
   info("Display the tooltip on box1.");
   await showTooltip(tooltip, box1, { x: 5, y: 10 });
 
   let panelRect = tooltip.container.getBoundingClientRect();
   let anchorRect = box1.getBoundingClientRect();
 
   // Tooltip will be displayed below box1
-  is(
-    panelRect.top,
-    anchorRect.bottom + 10 + offsetTop,
-    "Tooltip top has 10px offset"
-  );
-  is(
-    panelRect.left,
-    anchorRect.left + 5 + offsetLeft,
-    "Tooltip left has 5px offset"
-  );
+  is(panelRect.top, anchorRect.bottom + 10, "Tooltip top has 10px offset");
+  is(panelRect.left, anchorRect.left + 5, "Tooltip left has 5px offset");
   is(panelRect.height, 100, "Tooltip height is at 100px as expected");
 
   info("Display the tooltip on box2.");
   await showTooltip(tooltip, box2, { x: 5, y: 10 });
 
   panelRect = tooltip.container.getBoundingClientRect();
   anchorRect = box2.getBoundingClientRect();
 
   // Tooltip will be displayed below box2, but can't be fully displayed because of the
   // offset
-  is(
-    panelRect.top,
-    anchorRect.bottom + 10 + offsetTop,
-    "Tooltip top has 10px offset"
-  );
-  is(
-    panelRect.left,
-    anchorRect.left + 5 + offsetLeft,
-    "Tooltip left has 5px offset"
-  );
+  is(panelRect.top, anchorRect.bottom + 10, "Tooltip top has 10px offset");
+  is(panelRect.left, anchorRect.left + 5, "Tooltip left has 5px offset");
   is(panelRect.height, 90, "Tooltip height is only 90px");
 
   info("Display the tooltip on box3.");
   await showTooltip(tooltip, box3, { x: 5, y: 10 });
 
   panelRect = tooltip.container.getBoundingClientRect();
   anchorRect = box3.getBoundingClientRect();
 
   // Tooltip will be displayed above box3, but can't be fully displayed because of the
   // offset
   is(
     panelRect.bottom,
-    anchorRect.top - 10 + offsetTop,
+    anchorRect.top - 10,
     "Tooltip bottom is 10px above anchor"
   );
-  is(
-    panelRect.left,
-    anchorRect.left + 5 + offsetLeft,
-    "Tooltip left has 5px offset"
-  );
+  is(panelRect.left, anchorRect.left + 5, "Tooltip left has 5px offset");
   is(panelRect.height, 90, "Tooltip height is only 90px");
 
   info("Display the tooltip on box4.");
   await showTooltip(tooltip, box4, { x: 5, y: 10 });
 
   panelRect = tooltip.container.getBoundingClientRect();
   anchorRect = box4.getBoundingClientRect();
 
   // Tooltip will be displayed above box4
   is(
     panelRect.bottom,
-    anchorRect.top - 10 + offsetTop,
+    anchorRect.top - 10,
     "Tooltip bottom is 10px above anchor"
   );
-  is(
-    panelRect.left,
-    anchorRect.left + 5 + offsetLeft,
-    "Tooltip left has 5px offset"
-  );
+  is(panelRect.left, anchorRect.left + 5, "Tooltip left has 5px offset");
   is(panelRect.height, 100, "Tooltip height is at 100px as expected");
 
   await hideTooltip(tooltip);
 
   tooltip.destroy();
 });
--- a/devtools/client/shared/test/browser_html_tooltip_rtl.js
+++ b/devtools/client/shared/test/browser_html_tooltip_rtl.js
@@ -58,34 +58,28 @@ async function testRtlAnchors(doc, toolt
    * - box1 is aligned with the left edge of the toolbox
    * - box2 is displayed right after box1
    * - total toolbox width is 500px so each box is 125px wide
    */
 
   const box1 = doc.getElementById("box1");
   const box2 = doc.getElementById("box2");
 
-  const { offsetTop, offsetLeft } = getOffsets(tooltip.doc);
-
   info("Display the tooltip on box1.");
   await showTooltip(tooltip, box1, { position: "bottom" });
 
   let panelRect = tooltip.container.getBoundingClientRect();
   let anchorRect = box1.getBoundingClientRect();
 
   // box1 uses RTL direction, so the tooltip should be aligned with the right edge of the
   // anchor, but it is shifted to the right to fit in the toolbox.
-  is(
-    panelRect.left,
-    0 + offsetLeft,
-    "Tooltip is aligned with left edge of the toolbox"
-  );
+  is(panelRect.left, 0, "Tooltip is aligned with left edge of the toolbox");
   is(
     panelRect.top,
-    anchorRect.bottom + offsetTop,
+    anchorRect.bottom,
     "Tooltip aligned with the anchor bottom edge"
   );
   is(
     panelRect.height,
     TOOLTIP_HEIGHT,
     "Tooltip height is at 100px as expected"
   );
 
@@ -93,22 +87,22 @@ async function testRtlAnchors(doc, toolt
   await showTooltip(tooltip, box2, { position: "bottom" });
 
   panelRect = tooltip.container.getBoundingClientRect();
   anchorRect = box2.getBoundingClientRect();
 
   // box2 uses RTL direction, so the tooltip is aligned with the right edge of the anchor
   is(
     panelRect.right,
-    anchorRect.right + offsetLeft,
+    anchorRect.right,
     "Tooltip is aligned with right edge of anchor"
   );
   is(
     panelRect.top,
-    anchorRect.bottom + offsetTop,
+    anchorRect.bottom,
     "Tooltip aligned with the anchor bottom edge"
   );
   is(
     panelRect.height,
     TOOLTIP_HEIGHT,
     "Tooltip height is at 100px as expected"
   );
 }
@@ -127,33 +121,31 @@ async function testLtrAnchors(doc, toolt
    * - box3 is is displayed right after box2
    * - box4 is aligned with the right edge of the toolbox
    * - total toolbox width is 500px so each box is 125px wide
    */
 
   const box3 = doc.getElementById("box3");
   const box4 = doc.getElementById("box4");
 
-  const { offsetTop, offsetLeft } = getOffsets(tooltip.doc);
-
   info("Display the tooltip on box3.");
   await showTooltip(tooltip, box3, { position: "bottom" });
 
   let panelRect = tooltip.container.getBoundingClientRect();
   let anchorRect = box3.getBoundingClientRect();
 
   // box3 uses LTR direction, so the tooltip is aligned with the left edge of the anchor.
   is(
     panelRect.left,
-    anchorRect.left + offsetLeft,
+    anchorRect.left,
     "Tooltip is aligned with left edge of anchor"
   );
   is(
     panelRect.top,
-    anchorRect.bottom + offsetTop,
+    anchorRect.bottom,
     "Tooltip aligned with the anchor bottom edge"
   );
   is(
     panelRect.height,
     TOOLTIP_HEIGHT,
     "Tooltip height is at 100px as expected"
   );
 
@@ -162,22 +154,22 @@ async function testLtrAnchors(doc, toolt
 
   panelRect = tooltip.container.getBoundingClientRect();
   anchorRect = box4.getBoundingClientRect();
 
   // box4 uses LTR direction, so the tooltip should be aligned with the left edge of the
   // anchor, but it is shifted to the left to fit in the toolbox.
   is(
     panelRect.right,
-    TOOLBOX_WIDTH + offsetLeft,
+    TOOLBOX_WIDTH,
     "Tooltip is aligned with right edge of toolbox"
   );
   is(
     panelRect.top,
-    anchorRect.bottom + offsetTop,
+    anchorRect.bottom,
     "Tooltip aligned with the anchor bottom edge"
   );
   is(
     panelRect.height,
     TOOLTIP_HEIGHT,
     "Tooltip height is at 100px as expected"
   );
 }
--- a/devtools/client/shared/test/head.js
+++ b/devtools/client/shared/test/head.js
@@ -219,35 +219,8 @@ const showFilterPopupPresetsAndCreatePre
 
   onRender = widget.once("render");
   widget._savePreset({
     preventDefault: () => {},
   });
 
   await onRender;
 };
-
-/**
- * Our calculations are slightly off so we add offsets for hidpi and non-hidpi
- * screens.
- *
- * @param {Document} doc
- *        The document that owns the tooltip.
- */
-function getOffsets(doc) {
-  let offsetTop = 0;
-  let offsetLeft = 0;
-
-  if (doc && doc.defaultView.devicePixelRatio === 2) {
-    // On hidpi screens our calculations are off by 2 vertical pixels.
-    offsetTop = 2;
-  } else {
-    // On non-hidpi screens our calculations are off by 1 vertical pixel.
-    offsetTop = 1;
-  }
-
-  if (doc && doc.defaultView.devicePixelRatio !== 2) {
-    // On hidpi screens our calculations are off by 1 horizontal pixel.
-    offsetLeft = 1;
-  }
-
-  return { offsetTop, offsetLeft };
-}
--- a/devtools/client/shared/test/helper_html_tooltip.js
+++ b/devtools/client/shared/test/helper_html_tooltip.js
@@ -1,12 +1,11 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 /* eslint no-unused-vars: [2, {"vars": "local", "args": "none"}] */
-/* import-globals-from head.js */
 
 "use strict";
 
 /**
  * Helper methods for the HTMLTooltip integration tests.
  */
 
 /**
@@ -76,37 +75,28 @@ function waitForReflow(tooltip) {
 function checkTooltipGeometry(
   tooltip,
   anchor,
   { position, leftAligned = true, height, width } = {}
 ) {
   info("Check the tooltip geometry matches expected position and dimensions");
   const tooltipRect = tooltip.container.getBoundingClientRect();
   const anchorRect = anchor.getBoundingClientRect();
-  const { offsetTop, offsetLeft } = getOffsets(tooltip.doc);
 
   if (position === "top") {
-    is(
-      tooltipRect.bottom,
-      anchorRect.top + offsetTop,
-      "Tooltip is above the anchor"
-    );
+    is(tooltipRect.bottom, anchorRect.top, "Tooltip is above the anchor");
   } else if (position === "bottom") {
-    is(
-      tooltipRect.top,
-      anchorRect.bottom + offsetTop,
-      "Tooltip is below the anchor"
-    );
+    is(tooltipRect.top, anchorRect.bottom, "Tooltip is below the anchor");
   } else {
     ok(false, "Invalid position provided to checkTooltipGeometry");
   }
 
   if (leftAligned) {
     is(
       tooltipRect.left,
-      anchorRect.left + offsetLeft,
+      anchorRect.left,
       "Tooltip left-aligned with the anchor"
     );
   }
 
   is(tooltipRect.height, height, "Tooltip has the expected height");
   is(tooltipRect.width, width, "Tooltip has the expected width");
 }
--- a/devtools/client/shared/widgets/tooltip/HTMLTooltip.js
+++ b/devtools/client/shared/widgets/tooltip/HTMLTooltip.js
@@ -98,31 +98,28 @@ const EXTRA_BORDER = {
  *        Bounding rectangle for the viewport. top/left can be different from 0 if some
  *        space should not be used by tooltips (for instance OS toolbars, taskbars etc.).
  * @param {Number} height
  *        Preferred height for the tooltip.
  * @param {String} pos
  *        Preferred position for the tooltip. Possible values: "top" or "bottom".
  * @param {Number} offset
  *        Offset between the top of the anchor and the tooltip.
- * @param {Document} [doc]
- *        The current document (optional).
  * @return {Object}
  *         - {Number} top: the top offset for the tooltip.
  *         - {Number} height: the height to use for the tooltip container.
  *         - {String} computedPosition: Can differ from the preferred position depending
  *           on the available height). "top" or "bottom"
  */
 const calculateVerticalPosition = (
   anchorRect,
   viewportRect,
   height,
   pos,
-  offset,
-  doc = null
+  offset
 ) => {
   const { TOP, BOTTOM } = POSITION;
 
   let { top: anchorTop, height: anchorHeight } = anchorRect;
 
   // Translate to the available viewport space before calculating dimensions and position.
   anchorTop -= viewportRect.top;
 
@@ -150,24 +147,16 @@ const calculateVerticalPosition = (
   let top =
     pos === TOP
       ? anchorTop - height - offset
       : anchorTop + anchorHeight + offset;
 
   // Translate back to absolute coordinates by re-including viewport top margin.
   top += viewportRect.top;
 
-  if (doc && doc.defaultView.devicePixelRatio === 2) {
-    // On hidpi screens our calculations are off by 2 vertical pixels.
-    top += 2;
-  } else {
-    // On non-hidpi screens our calculations are off by 1 vertical pixel.
-    top += 1;
-  }
-
   return { top, height, computedPosition: pos };
 };
 
 /**
  * Calculate the horizontal position & offsets to use for the tooltip. Will
  * attempt to respect the provided width and position preferences, unless the
  * available width prevents this.
  *
@@ -188,34 +177,31 @@ const calculateVerticalPosition = (
  *        Horizontal offset in pixels.
  * @param {Number} borderRadius
  *        The border radius of the panel. This is added to ARROW_OFFSET to
  *        calculate the distance from the edge of the tooltip to the start
  *        of arrow. It is separate from ARROW_OFFSET since it will vary by
  *        platform.
  * @param {Boolean} isRtl
  *        If the anchor is in RTL, the tooltip should be aligned to the right.
- * @param {Document} [doc]
- *        The current document (optional).
  * @return {Object}
  *         - {Number} left: the left offset for the tooltip.
  *         - {Number} width: the width to use for the tooltip container.
  *         - {Number} arrowLeft: the left offset to use for the arrow element.
  */
 const calculateHorizontalPosition = (
   anchorRect,
   viewportRect,
   windowRect,
   width,
   type,
   offset,
   borderRadius,
   isRtl,
-  isMenuTooltip,
-  doc = null
+  isMenuTooltip
 ) => {
   // All tooltips from content should follow the writing direction.
   //
   // For tooltips (including doorhanger tooltips) we follow the writing
   // direction but for menus created using doorhangers the guidelines[1] say
   // that:
   //
   //   "Doorhangers opening on the right side of the view show the directional
@@ -276,30 +262,25 @@ const calculateHorizontalPosition = (
     // Translate the coordinate to tooltip container
     arrowStart += anchorStart - tooltipStart;
     // Make sure the arrow remains in the tooltip container.
     arrowStart = Math.min(arrowStart, tooltipWidth - arrowWidth - borderRadius);
     arrowStart = Math.max(arrowStart, borderRadius);
   }
 
   // Convert from logical coordinates to physical
-  let left =
+  const left =
     hangDirection === "right"
       ? viewportRect.left + tooltipStart
       : viewportRect.right - tooltipStart - tooltipWidth;
   const arrowLeft =
     hangDirection === "right"
       ? arrowStart
       : tooltipWidth - arrowWidth - arrowStart;
 
-  if (doc && doc.defaultView.devicePixelRatio !== 2) {
-    // On hidpi screens our calculations are off by 1 horizontal pixel.
-    left += 1;
-  }
-
   return { left, width: tooltipWidth, arrowLeft };
 };
 
 /**
  * Get the bounding client rectangle for a given node, relative to a custom
  * reference element (instead of the default for getBoundingClientRect which
  * is always the element's ownerDocument).
  */
@@ -601,18 +582,17 @@ HTMLTooltip.prototype = {
       anchorRect,
       viewportRect,
       windowRect,
       preferredWidth,
       this.type,
       x,
       borderRadius,
       isRtl,
-      this.isMenuTooltip,
-      this.doc
+      this.isMenuTooltip
     );
 
     // If we constrained the width, then any measured height we have is no
     // longer valid.
     if (measuredHeight && width !== preferredWidth) {
       measuredHeight = undefined;
     }
 
@@ -651,18 +631,17 @@ HTMLTooltip.prototype = {
       preferredHeight = this.preferredHeight + themeHeight;
     }
 
     const { top, height, computedPosition } = calculateVerticalPosition(
       anchorRect,
       viewportRect,
       preferredHeight,
       position,
-      y,
-      this.doc
+      y
     );
 
     this._position = computedPosition;
     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