Bug 1429763 - Show font preview when hovering on font names. r=pbro, a=lizzard
authorMichael Hoffmann <brennan.brisad@gmail.com>
Thu, 18 Jan 2018 14:33:00 +0200
changeset 452046 0f692cbb39d7b7a60900f34e79030fb69c094271
parent 452045 84a24eb373faf01707deb981a0d304c2fce9461f
child 452047 5798b4c1d8cd7752ab3c36c8c863c818aecf936f
push id8586
push userryanvm@gmail.com
push dateThu, 25 Jan 2018 02:57:25 +0000
treeherdermozilla-beta@4aedb56b41c7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspbro, lizzard
bugs1429763
milestone59.0
Bug 1429763 - Show font preview when hovering on font names. r=pbro, a=lizzard
devtools/client/inspector/computed/computed.js
devtools/client/inspector/rules/rules.js
devtools/client/inspector/rules/views/text-property-editor.js
devtools/client/inspector/shared/node-types.js
devtools/client/inspector/shared/test/browser_styleinspector_tooltip-longhand-fontfamily.js
devtools/client/inspector/shared/tooltips-overlay.js
--- a/devtools/client/inspector/computed/computed.js
+++ b/devtools/client/inspector/computed/computed.js
@@ -15,16 +15,17 @@ const {PrefObserver} = require("devtools
 const {createChild} = require("devtools/client/inspector/shared/utils");
 const {gDevTools} = require("devtools/client/framework/devtools");
 const {getCssProperties} = require("devtools/shared/fronts/css-properties");
 const {
   VIEW_NODE_SELECTOR_TYPE,
   VIEW_NODE_PROPERTY_TYPE,
   VIEW_NODE_VALUE_TYPE,
   VIEW_NODE_IMAGE_URL_TYPE,
+  VIEW_NODE_FONT_TYPE,
 } = require("devtools/client/inspector/shared/node-types");
 const StyleInspectorMenu = require("devtools/client/inspector/shared/style-inspector-menu");
 const TooltipsOverlay = require("devtools/client/inspector/shared/tooltips-overlay");
 const KeyShortcuts = require("devtools/client/shared/key-shortcuts");
 const clipboardHelper = require("devtools/shared/platform/clipboard");
 
 const { createElement, createFactory } = require("devtools/client/shared/vendor/react");
 const ReactDOM = require("devtools/client/shared/vendor/react-dom");
@@ -369,23 +370,42 @@ CssComputedView.prototype = {
     if (propertyContent && (classes.contains("computed-other-property-value") ||
                             isHref)) {
       let view = propertyContent.previousSibling;
       value = {
         property: view.querySelector(".computed-property-name").firstChild.textContent,
         value: node.textContent
       };
     }
+    if (classes.contains("computed-font-family")) {
+      if (propertyView) {
+        value = {
+          property: parent.querySelector(
+            ".computed-property-name").firstChild.textContent,
+          value: node.parentNode.textContent
+        };
+      } else if (propertyContent) {
+        let view = propertyContent.previousSibling;
+        value = {
+          property: view.querySelector(".computed-property-name").firstChild.textContent,
+          value: node.parentNode.textContent
+        };
+      } else {
+        return null;
+      }
+    }
 
     // Get the type
     if (classes.contains("computed-property-name")) {
       type = VIEW_NODE_PROPERTY_TYPE;
     } else if (classes.contains("computed-property-value") ||
                classes.contains("computed-other-property-value")) {
       type = VIEW_NODE_VALUE_TYPE;
+    } else if (classes.contains("computed-font-family")) {
+      type = VIEW_NODE_FONT_TYPE;
     } else if (isHref) {
       type = VIEW_NODE_IMAGE_URL_TYPE;
       value.url = node.href;
     } else {
       return null;
     }
 
     return {
@@ -1055,17 +1075,18 @@ PropertyView.prototype = {
     this.tree.numVisibleProperties++;
 
     let outputParser = this.tree._outputParser;
     let frag = outputParser.parseCssProperty(this.propertyInfo.name,
       this.propertyInfo.value,
       {
         colorSwatchClass: "computed-colorswatch",
         colorClass: "computed-color",
-        urlClass: "theme-link"
+        urlClass: "theme-link",
+        fontFamilyClass: "computed-font-family",
         // No need to use baseURI here as computed URIs are never relative.
       });
     this.valueNode.innerHTML = "";
     this.valueNode.appendChild(frag);
 
     this.refreshMatchedSelectors();
   },
 
@@ -1329,16 +1350,17 @@ SelectorView.prototype = {
     // templater.
     let outputParser = this.tree._outputParser;
     let frag = outputParser.parseCssProperty(
       this.selectorInfo.name,
       this.selectorInfo.value, {
         colorSwatchClass: "computed-colorswatch",
         colorClass: "computed-color",
         urlClass: "theme-link",
+        fontFamilyClass: "computed-font-family",
         baseURI: this.selectorInfo.rule.href
       }
     );
     return frag;
   },
 
   /**
    * Update the text of the source link to reflect whether we're showing
--- a/devtools/client/inspector/rules/rules.js
+++ b/devtools/client/inspector/rules/rules.js
@@ -21,16 +21,17 @@ const {getCssProperties} = require("devt
 const {
   VIEW_NODE_SELECTOR_TYPE,
   VIEW_NODE_PROPERTY_TYPE,
   VIEW_NODE_VALUE_TYPE,
   VIEW_NODE_IMAGE_URL_TYPE,
   VIEW_NODE_LOCATION_TYPE,
   VIEW_NODE_SHAPE_POINT_TYPE,
   VIEW_NODE_VARIABLE_TYPE,
+  VIEW_NODE_FONT_TYPE,
 } = require("devtools/client/inspector/shared/node-types");
 const StyleInspectorMenu = require("devtools/client/inspector/shared/style-inspector-menu");
 const TooltipsOverlay = require("devtools/client/inspector/shared/tooltips-overlay");
 const {createChild, promiseWarn} = require("devtools/client/inspector/shared/utils");
 const {debounce} = require("devtools/shared/debounce");
 const EventEmitter = require("devtools/shared/old-event-emitter");
 const KeyShortcuts = require("devtools/client/shared/key-shortcuts");
 const clipboardHelper = require("devtools/shared/platform/clipboard");
@@ -322,16 +323,27 @@ CssRuleView.prototype = {
         property: getPropertyNameAndValue(node).name,
         value: node.textContent,
         enabled: prop.enabled,
         overridden: prop.overridden,
         pseudoElement: prop.rule.pseudoElement,
         sheetHref: prop.rule.domRule.href,
         textProperty: prop
       };
+    } else if (classes.contains("ruleview-font-family") && prop) {
+      type = VIEW_NODE_FONT_TYPE;
+      value = {
+        property: getPropertyNameAndValue(node).name,
+        value: getPropertyNameAndValue(node).value,
+        enabled: prop.enabled,
+        overridden: prop.overridden,
+        pseudoElement: prop.rule.pseudoElement,
+        sheetHref: prop.rule.domRule.href,
+        textProperty: prop
+      };
     } else if (classes.contains("ruleview-shape-point") && prop) {
       type = VIEW_NODE_SHAPE_POINT_TYPE;
       value = {
         property: getPropertyNameAndValue(node).name,
         value: node.textContent,
         enabled: prop.enabled,
         overridden: prop.overridden,
         pseudoElement: prop.rule.pseudoElement,
--- a/devtools/client/inspector/rules/views/text-property-editor.js
+++ b/devtools/client/inspector/rules/views/text-property-editor.js
@@ -673,17 +673,18 @@ TextPropertyEditor.prototype = {
     });
     appendText(li, ": ");
 
     let outputParser = this.ruleView._outputParser;
     let frag = outputParser.parseCssProperty(
       computed.name, computed.value, {
         colorSwatchClass: "ruleview-swatch ruleview-colorswatch",
         urlClass: "theme-link",
-        baseURI: this.sheetHref
+        baseURI: this.sheetHref,
+        fontFamilyClass: "ruleview-font-family"
       }
     );
 
     // Store the computed property value that was parsed for output
     computed.parsedValue = frag.textContent;
 
     createChild(li, "span", {
       class: "ruleview-propertyvalue theme-fg-color1",
--- a/devtools/client/inspector/shared/node-types.js
+++ b/devtools/client/inspector/shared/node-types.js
@@ -12,8 +12,9 @@
 
 exports.VIEW_NODE_SELECTOR_TYPE = 1;
 exports.VIEW_NODE_PROPERTY_TYPE = 2;
 exports.VIEW_NODE_VALUE_TYPE = 3;
 exports.VIEW_NODE_IMAGE_URL_TYPE = 4;
 exports.VIEW_NODE_LOCATION_TYPE = 5;
 exports.VIEW_NODE_SHAPE_POINT_TYPE = 6;
 exports.VIEW_NODE_VARIABLE_TYPE = 7;
+exports.VIEW_NODE_FONT_TYPE = 8;
--- a/devtools/client/inspector/shared/test/browser_styleinspector_tooltip-longhand-fontfamily.js
+++ b/devtools/client/inspector/shared/test/browser_styleinspector_tooltip-longhand-fontfamily.js
@@ -57,16 +57,34 @@ function* testRuleView(ruleView, nodeFro
   ok(images[0].getAttribute("src").startsWith("data:"),
     "Tooltip contains a data-uri image as expected");
 
   let dataURL = yield getFontFamilyDataURL(valueSpan.textContent, nodeFront);
   is(images[0].getAttribute("src"), dataURL,
     "Tooltip contains the correct data-uri image");
 
   yield assertTooltipHiddenOnMouseOut(previewTooltip, valueSpan);
+
+  // Do the tooltip test again, but now when hovering on the span that
+  // encloses each and every font family.
+  const fontFamilySpan = valueSpan.querySelector(".ruleview-font-family");
+  fontFamilySpan.scrollIntoView(true);
+
+  previewTooltip = yield assertShowPreviewTooltip(ruleView, fontFamilySpan);
+
+  images = panel.getElementsByTagName("img");
+  is(images.length, 1, "Tooltip contains an image");
+  ok(images[0].getAttribute("src").startsWith("data:"),
+    "Tooltip contains a data-uri image as expected");
+
+  dataURL = yield getFontFamilyDataURL(fontFamilySpan.textContent, nodeFront);
+  is(images[0].getAttribute("src"), dataURL,
+    "Tooltip contains the correct data-uri image");
+
+  yield assertTooltipHiddenOnMouseOut(previewTooltip, fontFamilySpan);
 }
 
 function* testComputedView(computedView, nodeFront) {
   info("Testing font-family tooltips in the computed view");
 
   let tooltip = computedView.tooltips.getTooltip("previewTooltip");
   let panel = tooltip.panel;
   let {valueSpan} = getComputedViewProperty(computedView, "font-family");
--- a/devtools/client/inspector/shared/tooltips-overlay.js
+++ b/devtools/client/inspector/shared/tooltips-overlay.js
@@ -10,16 +10,17 @@
  * The tooltip overlays are tooltips that appear when hovering over property values and
  * editor tooltips that appear when clicking swatch based editors.
  */
 
 const { Task } = require("devtools/shared/task");
 const Services = require("Services");
 const {
   VIEW_NODE_VALUE_TYPE,
+  VIEW_NODE_FONT_TYPE,
   VIEW_NODE_IMAGE_URL_TYPE,
   VIEW_NODE_VARIABLE_TYPE,
 } = require("devtools/client/inspector/shared/node-types");
 const { getColor } = require("devtools/client/shared/theme");
 const { HTMLTooltip } = require("devtools/client/shared/widgets/tooltip/HTMLTooltip");
 
 loader.lazyRequireGetter(this, "getCssProperties",
   "devtools/shared/fronts/css-properties", true);
@@ -166,17 +167,18 @@ TooltipsOverlay.prototype = {
 
     // Image preview tooltip
     if (type === VIEW_NODE_IMAGE_URL_TYPE &&
         inspector.hasUrlToImageDataResolver) {
       tooltipType = TOOLTIP_IMAGE_TYPE;
     }
 
     // Font preview tooltip
-    if (type === VIEW_NODE_VALUE_TYPE && prop.property === "font-family") {
+    if ((type === VIEW_NODE_VALUE_TYPE && prop.property === "font-family") ||
+        (type === VIEW_NODE_FONT_TYPE)) {
       let value = prop.value.toLowerCase();
       if (value !== "inherit" && value !== "unset" && value !== "initial") {
         tooltipType = TOOLTIP_FONTFAMILY_TYPE;
       }
     }
 
     // Variable preview tooltip
     if (type === VIEW_NODE_VARIABLE_TYPE) {
@@ -226,16 +228,22 @@ TooltipsOverlay.prototype = {
       }
       return true;
     }
 
     if (type === TOOLTIP_FONTFAMILY_TYPE) {
       let font = nodeInfo.value.value;
       let nodeFront = inspector.selection.nodeFront;
       yield this._setFontPreviewTooltip(font, nodeFront);
+
+      if (nodeInfo.type === VIEW_NODE_FONT_TYPE) {
+        // If the hovered element is on the font family span, anchor
+        // the tooltip on the whole property value instead.
+        return target.parentNode;
+      }
       return true;
     }
 
     if (type === TOOLTIP_VARIABLE_TYPE && nodeInfo.value.value.startsWith("--")) {
       let variable = nodeInfo.value.variable;
       yield this._setVariablePreviewTooltip(variable);
       return true;
     }