Bug 1501918 - Restore tooltip size when updating Variable and BrokenImage content;r=pbro
authorJulian Descottes <jdescottes@mozilla.com>
Thu, 25 Oct 2018 09:44:18 +0000
changeset 491302 95fc220097602f6527926c706877497d00b61c86
parent 491301 88351e371a8b8c21dc69555f53707a8fae6df40a
child 491303 d4b2fecf181c6c89a205eaf0058bbf129f780faa
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewerspbro
bugs1501918
milestone65.0a1
Bug 1501918 - Restore tooltip size when updating Variable and BrokenImage content;r=pbro Another regression linked to removing setContent API on HTMLTooltip. The initial feedback to remove the method was from me, because we started seeing two ways to set the content of the tooltip (DOM APIs or setContent) and I prefered keeping only one. However the DOM approach still almost forces you to call setContentSize in case your tooltip instance is shared for different content. This is the case for the preview tooltip, which is used for CSS variables, fonts and image previews. Maybe we should revisit the decision to remove this setContent API Differential Revision: https://phabricator.services.mozilla.com/D9752
devtools/client/inspector/rules/test/browser.ini
devtools/client/inspector/rules/test/browser_rules_preview-tooltips-sizes.js
devtools/client/shared/widgets/tooltip/ImageTooltipHelper.js
devtools/client/shared/widgets/tooltip/VariableTooltipHelper.js
--- a/devtools/client/inspector/rules/test/browser.ini
+++ b/devtools/client/inspector/rules/test/browser.ini
@@ -213,16 +213,17 @@ skip-if = (os == "win" && debug) # bug 9
 [browser_rules_multiple-properties-priority.js]
 [browser_rules_multiple-properties-unfinished_01.js]
 [browser_rules_multiple-properties-unfinished_02.js]
 [browser_rules_multiple_properties_01.js]
 [browser_rules_multiple_properties_02.js]
 [browser_rules_non_ascii.js]
 [browser_rules_original-source-link.js]
 [browser_rules_original-source-link2.js]
+[browser_rules_preview-tooltips-sizes.js]
 [browser_rules_pseudo-element_01.js]
 [browser_rules_pseudo-element_02.js]
 [browser_rules_pseudo_lock_options.js]
 [browser_rules_refresh-no-flicker.js]
 [browser_rules_refresh-on-attribute-change_01.js]
 [browser_rules_refresh-on-style-change.js]
 [browser_rules_search-filter-computed-list_01.js]
 [browser_rules_search-filter-computed-list_02.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/rules/test/browser_rules_preview-tooltips-sizes.js
@@ -0,0 +1,69 @@
+/* vim: set ft=javascript ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Test that the dimensions of the preview tooltips are correctly updated to fit their
+// content.
+
+// Small 32x32 image.
+const BASE_64_URL = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAACAAAAAgCAYAAABzenr" +
+  "0AAAAUElEQVRYR+3UsQkAQAhD0TjJ7T+Wk3gbxMIizbcVITwwJWlkZtptpXp+v94TAAEE4gLTvgfOf770RB" +
+  "EAAQTiAvEiIgACCMQF4kVEAAQQSAt8xsyeAW6R8eIAAAAASUVORK5CYII=";
+
+add_task(async function() {
+  await addTab("data:text/html;charset=utf-8," +
+    encodeURIComponent(`
+      <style>
+        html {
+          /* Using a long variable name to ensure preview tooltip for variable will be */
+          /* wider than the preview tooltip for the test 32x32 image. */
+          --test-var-wider-than-image: red;
+        }
+
+        div {
+          color: var(--test-var-wider-than-image);
+          background: url(${BASE_64_URL});
+        }
+      </style>
+      <div id="target">inspect me</div>
+    `));
+  const {inspector, view} = await openRuleView();
+  await selectNode("#target", inspector);
+
+  // Retrieve the element for `--test-var` on which the CSS variable tooltip will appear.
+  const colorPropertySpan = getRuleViewProperty(view, "div", "color").valueSpan;
+  const colorVariableElement = colorPropertySpan.querySelector(".ruleview-variable");
+
+  // Retrieve the element for the background url on which the image preview will appear.
+  const backgroundPropertySpan = getRuleViewProperty(view, "div", "background").valueSpan;
+  const backgroundUrlElement = backgroundPropertySpan.querySelector(".theme-link");
+
+  info("Show preview tooltip for CSS variable");
+  let previewTooltip = await assertShowPreviewTooltip(view, colorVariableElement);
+  // Measure tooltip dimensions.
+  let tooltipRect = previewTooltip.panel.getBoundingClientRect();
+  const originalHeight = tooltipRect.height;
+  const originalWidth = tooltipRect.width;
+  info(`Original dimensions: ${originalWidth} x ${originalHeight}`);
+  await assertTooltipHiddenOnMouseOut(previewTooltip, colorVariableElement);
+
+  info("Show preview tooltip for background url");
+  previewTooltip = await assertShowPreviewTooltip(view, backgroundUrlElement);
+  // Compare new tooltip dimensions to previous measures.
+  tooltipRect = previewTooltip.panel.getBoundingClientRect();
+  info(`Image preview dimensions: ${tooltipRect.width} x ${tooltipRect.height}`);
+  ok(tooltipRect.height > originalHeight, "Tooltip is taller for image preview");
+  ok(tooltipRect.width < originalWidth, "Tooltip is narrower for image preview");
+  await assertTooltipHiddenOnMouseOut(previewTooltip, colorVariableElement);
+
+  info("Show preview tooltip for CSS variable again");
+  previewTooltip = await assertShowPreviewTooltip(view, colorVariableElement);
+  // Check measures are identical to initial ones.
+  tooltipRect = previewTooltip.panel.getBoundingClientRect();
+  info(`CSS variable tooltip dimensions: ${tooltipRect.width} x ${tooltipRect.height}`);
+  is(tooltipRect.height, originalHeight, "Tooltip has the same height as the original");
+  is(tooltipRect.width, originalWidth, "Tooltip has the same width as the original");
+  await assertTooltipHiddenOnMouseOut(previewTooltip, colorVariableElement);
+});
--- a/devtools/client/shared/widgets/tooltip/ImageTooltipHelper.js
+++ b/devtools/client/shared/widgets/tooltip/ImageTooltipHelper.js
@@ -133,13 +133,14 @@ function setImageTooltip(tooltip, doc, i
 function setBrokenImageTooltip(tooltip, doc) {
   const div = doc.createElementNS(XHTML_NS, "div");
   div.className = "theme-comment devtools-tooltip-image-broken";
   const message = L10N.getStr("previewTooltip.image.brokenImage");
   div.textContent = message;
 
   tooltip.panel.innerHTML = "";
   tooltip.panel.appendChild(div);
+  tooltip.setContentSize({width: "auto", height: "auto"});
 }
 
 module.exports.getImageDimensions = getImageDimensions;
 module.exports.setImageTooltip = setImageTooltip;
 module.exports.setBrokenImageTooltip = setBrokenImageTooltip;
--- a/devtools/client/shared/widgets/tooltip/VariableTooltipHelper.js
+++ b/devtools/client/shared/widgets/tooltip/VariableTooltipHelper.js
@@ -20,11 +20,12 @@ const XHTML_NS = "http://www.w3.org/1999
 function setVariableTooltip(tooltip, doc, text) {
   // Create tooltip content
   const div = doc.createElementNS(XHTML_NS, "div");
   div.classList.add("devtools-monospace", "devtools-tooltip-css-variable");
   div.textContent = text;
 
   tooltip.panel.innerHTML = "";
   tooltip.panel.appendChild(div);
+  tooltip.setContentSize({width: "auto", height: "auto"});
 }
 
 module.exports.setVariableTooltip = setVariableTooltip;