Bug 1461522 - Make HTMLTooltip position the arrow correctly in RTL mode; r?jdescottes draft
authorBrian Birtles <birtles@gmail.com>
Thu, 28 Jun 2018 14:52:10 +0900
changeset 813894 b0dfde84ac8da67818e101fb132a278e433dd33d
parent 813837 987ea0d6a000b95cf93928b25a74a7fb1dfe37b2
child 813895 9f707175c1b9aaa8fca749123329e6d307c82668
push id115042
push userbbirtles@mozilla.com
push dateWed, 04 Jul 2018 04:36:27 +0000
reviewersjdescottes
bugs1461522
milestone63.0a1
Bug 1461522 - Make HTMLTooltip position the arrow correctly in RTL mode; r?jdescottes Currently the arrow offset is not correctly positioned in RTL mode as evidenced by the test case included in this patch (which fails without the code changes included in this patch). This patch logicalizes the calculations for positioning the tooltip which fixes this and other issues when using RTL mode and will make it easier to introduce a new "doorhanger" type tooltip later in this patch series. MozReview-Commit-ID: BQXhDHbPTbl
devtools/client/shared/test/browser_html_tooltip_rtl.js
devtools/client/shared/widgets/tooltip/HTMLTooltip.js
--- a/devtools/client/shared/test/browser_html_tooltip_rtl.js
+++ b/devtools/client/shared/test/browser_html_tooltip_rtl.js
@@ -19,29 +19,31 @@ const TOOLBOX_WIDTH = 500;
 const TOOLTIP_WIDTH = 150;
 const TOOLTIP_HEIGHT = 30;
 
 add_task(async function() {
   await pushPref("devtools.toolbox.sidebar.width", TOOLBOX_WIDTH);
 
   const [,, doc] = await createHost("right", TEST_URI);
 
-  info("Test a tooltip is not closed when clicking inside itself");
+  info("Test the positioning of tooltips in RTL and LTR directions");
 
   const tooltip = new HTMLTooltip(doc, {useXulWrapper: false});
   const div = doc.createElementNS(HTML_NS, "div");
   div.textContent = "tooltip";
   div.style.cssText = "box-sizing: border-box; border: 1px solid black";
   tooltip.setContent(div, {width: TOOLTIP_WIDTH, height: TOOLTIP_HEIGHT});
 
   await testRtlAnchors(doc, tooltip);
   await testLtrAnchors(doc, tooltip);
   await hideTooltip(tooltip);
 
   tooltip.destroy();
+
+  await testRtlArrow(doc);
 });
 
 async function testRtlAnchors(doc, tooltip) {
   /*
    * The layout of the test page is as follows:
    *   _______________________________
    *  | toolbox                       |
    *  | _____   _____   _____   _____ |
@@ -119,8 +121,48 @@ async function testLtrAnchors(doc, toolt
   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, "Tooltip is aligned with right edge of toolbox");
   is(panelRect.top, anchorRect.bottom, "Tooltip aligned with the anchor bottom edge");
   is(panelRect.height, TOOLTIP_HEIGHT, "Tooltip height is at 100px as expected");
 }
+
+async function testRtlArrow(doc) {
+  // Set up the arrow-style tooltip
+  const arrowTooltip =
+    new HTMLTooltip(doc, { type: "arrow", useXulWrapper: false });
+  const div = doc.createElementNS(HTML_NS, "div");
+  div.textContent = "tooltip";
+  div.style.cssText = "box-sizing: border-box; border: 1px solid black";
+  arrowTooltip.setContent(div, {
+    width: TOOLTIP_WIDTH,
+    height: TOOLTIP_HEIGHT,
+  });
+
+  // box2 uses RTL direction and is far enough from the edge that the arrow
+  // should not be squashed in the wrong direction.
+  const box2 = doc.getElementById("box2");
+
+  info("Display the arrow tooltip on box2.");
+  await showTooltip(arrowTooltip, box2, { position: "top" });
+
+  const arrow = arrowTooltip.arrow;
+  ok(arrow, "Tooltip has an arrow");
+
+  const panelRect = arrowTooltip.container.getBoundingClientRect();
+  const arrowRect = arrow.getBoundingClientRect();
+
+  // The arrow should be offset from the right edge, but still closer to the
+  // right edge than the left edge.
+  ok(arrowRect.right < panelRect.right, "Right edge of the arrow " +
+     `(${arrowRect.right}) is less than the right edge of the panel ` +
+     `(${panelRect.right})`);
+  const rightMargin = panelRect.right - arrowRect.right;
+  const leftMargin = arrowRect.left - panelRect.right;
+  ok(rightMargin > leftMargin, "Arrow should be closer to the right side of " +
+     ` the panel (margin: ${rightMargin}) than the left side ` +
+     ` (margin: ${leftMargin})`);
+
+  await hideTooltip(arrowTooltip);
+  arrowTooltip.destroy();
+}
--- a/devtools/client/shared/widgets/tooltip/HTMLTooltip.js
+++ b/devtools/client/shared/widgets/tooltip/HTMLTooltip.js
@@ -98,79 +98,100 @@ function(anchorRect, viewportRect, heigh
 
   // Translate back to absolute coordinates by re-including viewport top margin.
   top += viewportRect.top;
 
   return {top, height, computedPosition: pos};
 };
 
 /**
- * Calculate the vertical position & offsets to use for the tooltip. Will attempt to
- * respect the provided height and position preferences, unless the available height
- * prevents this.
+ * 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.
  *
  * @param {DOMRect} anchorRect
  *        Bounding rectangle for the anchor, relative to the tooltip document.
  * @param {DOMRect} viewportRect
- *        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.).
+ *        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} width
  *        Preferred width for the tooltip.
  * @param {String} type
  *        The tooltip type (e.g. "arrow").
  * @param {Number} offset
  *        Horizontal offset in pixels.
  * @param {Boolean} isRtl
  *        If the anchor is in RTL, the tooltip should be aligned to the right.
  * @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 =
-function(anchorRect, viewportRect, width, type, offset, isRtl) {
+const calculateHorizontalPosition = (
+  anchorRect,
+  viewportRect,
+  width,
+  type,
+  offset,
+  isRtl
+) => {
+  // Which direction should the tooltip go?
+  const hangDirection = isRtl ? "left" : "right";
   const anchorWidth = anchorRect.width;
-  let anchorStart = isRtl ? anchorRect.right : anchorRect.left;
 
-  // Translate to the available viewport space before calculating dimensions and position.
-  anchorStart -= viewportRect.left;
-
-  // Calculate WIDTH.
-  width = Math.min(width, viewportRect.width);
+  // Calculate logical start of anchor relative to the viewport.
+  const anchorStart =
+    hangDirection === "right"
+      ? anchorRect.left - viewportRect.left
+      : viewportRect.right - anchorRect.right;
 
-  // Calculate LEFT.
-  // By default the tooltip is aligned with the anchor left edge. Unless this
-  // makes it overflow the viewport, in which case is shifts to the left.
-  let left = anchorStart + offset - (isRtl ? width : 0);
-  left = Math.min(left, viewportRect.width - width);
-  left = Math.max(0, left);
+  // Calculate tooltip width.
+  const tooltipWidth = Math.min(width, viewportRect.width);
+
+  // Calculate tooltip start.
+  let tooltipStart = anchorStart + offset;
+  tooltipStart = Math.min(tooltipStart, viewportRect.width - tooltipWidth);
+  tooltipStart = Math.max(0, tooltipStart);
 
-  // Calculate ARROW LEFT (tooltip's LEFT might be updated)
-  let arrowLeft;
-  // Arrow style tooltips may need to be shifted to the left
+  // Calculate arrow start (tooltip's start might be updated)
+  const arrowWidth = type === TYPE.ARROW ? ARROW_WIDTH : 0;
+  let arrowStart;
+  // Arrow style tooltips may need to be shifted
   if (type === TYPE.ARROW) {
-    const arrowCenter = left + ARROW_OFFSET + ARROW_WIDTH / 2;
+    // Where will the point of the arrow be if we apply the standard offset?
+    const arrowCenter = tooltipStart + ARROW_OFFSET + arrowWidth / 2;
+
+    // How does that compare to the center of the anchor?
     const anchorCenter = anchorStart + anchorWidth / 2;
+
     // If the anchor is too narrow, align the arrow and the anchor center.
     if (arrowCenter > anchorCenter) {
-      left = Math.max(0, left - (arrowCenter - anchorCenter));
+      tooltipStart = Math.max(0, tooltipStart - (arrowCenter - anchorCenter));
     }
-    // Arrow's left offset relative to the anchor.
-    arrowLeft = Math.min(ARROW_OFFSET, (anchorWidth - ARROW_WIDTH) / 2) | 0;
+    // Arrow's start offset relative to the anchor.
+    arrowStart = Math.min(ARROW_OFFSET, (anchorWidth - arrowWidth) / 2) | 0;
     // Translate the coordinate to tooltip container
-    arrowLeft += anchorStart - left;
+    arrowStart += anchorStart - tooltipStart;
     // Make sure the arrow remains in the tooltip container.
-    arrowLeft = Math.min(arrowLeft, width - ARROW_WIDTH);
-    arrowLeft = Math.max(arrowLeft, 0);
+    arrowStart = Math.min(arrowStart, tooltipWidth - arrowWidth);
+    arrowStart = Math.max(arrowStart, 0);
   }
 
-  // Translate back to absolute coordinates by re-including viewport left margin.
-  left += viewportRect.left;
+  // Convert from logical coordinates to physical
+  const left =
+    hangDirection === "right"
+      ? viewportRect.left + tooltipStart
+      : viewportRect.right - tooltipStart - tooltipWidth;
+  const arrowLeft =
+    hangDirection === "right"
+      ? arrowStart
+      : tooltipWidth - arrowWidth - arrowStart;
 
-  return {left, width, arrowLeft};
+  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).
  */
 const getRelativeRect = function(node, relativeTo) {