Bug 1440855 - New font text-run highlighter used from the font inspector;r=gl draft
authorPatrick Brosset <pbrosset@mozilla.com>
Wed, 02 May 2018 14:44:30 +0200
changeset 792356 80493e64ad6fdb1ec1963021eb9bd7c3f5398201
parent 792270 59005ba3cd3e7b3f9e8804bea881bf4c3a755d7c
push id109084
push userbmo:pbrosset@mozilla.com
push dateTue, 08 May 2018 08:23:59 +0000
reviewersgl
bugs1440855
milestone62.0a1
Bug 1440855 - New font text-run highlighter used from the font inspector;r=gl This commit introduces a new highlighter. This highlighter is specialized in highlighting text-runs in a page that use a specified font. The highlighter is based on a platform API that returns Range objects. Therefore, the approach I chose was to simply feed these objects to the window Selection object. This way, we get highlighting for free without having to create any markup in the content page. The drawback is that the highlighting looks different than in other places of DevTools. However it's most probably way better in terms of performance, and will adapt natively to edge cases like APZ, scrolling, CSS transform, etc. This commit also has a simple UI: on mouseover of a font name in the font inspector panel, corresponding text runs are highlighted in the page. This UI is hidden behind a pref for now, so we can test the feature with some chosen users and gather feedback to improve the UI before shipping. Finally, an integration test was added. MozReview-Commit-ID: Fu3t0b5kbdy
devtools/client/inspector/fonts/components/Font.js
devtools/client/inspector/fonts/components/FontList.js
devtools/client/inspector/fonts/components/FontMeta.js
devtools/client/inspector/fonts/components/FontOverview.js
devtools/client/inspector/fonts/components/FontsApp.js
devtools/client/inspector/fonts/fonts.js
devtools/client/inspector/fonts/test/browser.ini
devtools/client/inspector/fonts/test/browser_fontinspector_reveal-in-page.js
devtools/client/inspector/fonts/test/head.js
devtools/client/preferences/devtools-client.js
devtools/server/actors/highlighters.js
devtools/server/actors/highlighters/fonts.js
devtools/server/actors/highlighters/moz.build
--- a/devtools/client/inspector/fonts/components/Font.js
+++ b/devtools/client/inspector/fonts/components/Font.js
@@ -13,17 +13,19 @@ const FontPreview = createFactory(requir
 
 const Types = require("../types");
 
 class Font extends PureComponent {
   static get propTypes() {
     return {
       font: PropTypes.shape(Types.font).isRequired,
       fontOptions: PropTypes.shape(Types.fontOptions).isRequired,
+      isForCurrentElement: PropTypes.bool.isRequired,
       onPreviewFonts: PropTypes.func.isRequired,
+      onToggleFontHighlight: PropTypes.func.isRequired,
     };
   }
 
   constructor(props) {
     super(props);
 
     this.state = {
       isFontFaceRuleExpanded: false,
@@ -94,31 +96,33 @@ class Font extends PureComponent {
 
     return dom.span(attributes);
   }
 
   render() {
     let {
       font,
       fontOptions,
+      isForCurrentElement,
       onPreviewFonts,
+      onToggleFontHighlight,
     } = this.props;
 
     let { previewText } = fontOptions;
 
     let {
       previewUrl,
       rule,
       ruleText,
     } = font;
 
     return dom.li(
       {
         className: "font",
       },
-      FontMeta({ font }),
+      FontMeta({ font, isForCurrentElement, onToggleFontHighlight }),
       FontPreview({ previewText, previewUrl, onPreviewFonts }),
       this.renderFontCSSCode(rule, ruleText)
     );
   }
 }
 
 module.exports = Font;
--- a/devtools/client/inspector/fonts/components/FontList.js
+++ b/devtools/client/inspector/fonts/components/FontList.js
@@ -12,34 +12,40 @@ const Font = createFactory(require("./Fo
 
 const Types = require("../types");
 
 class FontList extends PureComponent {
   static get propTypes() {
     return {
       fontOptions: PropTypes.shape(Types.fontOptions).isRequired,
       fonts: PropTypes.arrayOf(PropTypes.shape(Types.font)).isRequired,
+      isCurrentElementFonts: PropTypes.bool.isRequired,
       onPreviewFonts: PropTypes.func.isRequired,
+      onToggleFontHighlight: PropTypes.func.isRequired,
     };
   }
 
   render() {
     let {
       fonts,
       fontOptions,
-      onPreviewFonts
+      isCurrentElementFonts,
+      onPreviewFonts,
+      onToggleFontHighlight
     } = this.props;
 
     return dom.ul(
       {
         className: "fonts-list"
       },
       fonts.map((font, i) => Font({
         key: i,
         font,
         fontOptions,
+        isForCurrentElement: isCurrentElementFonts,
         onPreviewFonts,
+        onToggleFontHighlight,
       }))
     );
   }
 }
 
 module.exports = FontList;
--- a/devtools/client/inspector/fonts/components/FontMeta.js
+++ b/devtools/client/inspector/fonts/components/FontMeta.js
@@ -7,34 +7,61 @@
 const { createElement, Fragment, PureComponent } =
   require("devtools/client/shared/vendor/react");
 const dom = require("devtools/client/shared/vendor/react-dom-factories");
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
 
 loader.lazyRequireGetter(this, "clipboardHelper", "devtools/shared/platform/clipboard");
 
 const { getStr } = require("../utils/l10n");
+const Services = require("Services");
 const Types = require("../types");
 
+const FONT_HIGHLIGHTER_PREF = "devtools.inspector.fonthighlighter.enabled";
+
 class FontMeta extends PureComponent {
   static get propTypes() {
     return {
       font: PropTypes.shape(Types.font).isRequired,
+      isForCurrentElement: PropTypes.bool.isRequired,
+      onToggleFontHighlight: PropTypes.func.isRequired,
     };
   }
 
   constructor(props) {
     super(props);
     this.onCopyURL = this.onCopyURL.bind(this);
+    this.onNameMouseOver = this.onNameMouseOver.bind(this);
+    this.onNameMouseOut = this.onNameMouseOut.bind(this);
   }
 
   onCopyURL() {
     clipboardHelper.copyString(this.props.font.URI);
   }
 
+  onNameMouseOver() {
+    let {
+      font,
+      isForCurrentElement,
+      onToggleFontHighlight,
+    } = this.props;
+
+    onToggleFontHighlight(font, true, isForCurrentElement);
+  }
+
+  onNameMouseOut() {
+    let {
+      font,
+      isForCurrentElement,
+      onToggleFontHighlight,
+    } = this.props;
+
+    onToggleFontHighlight(font, false, isForCurrentElement);
+  }
+
   renderFontOrigin(url) {
     if (!url) {
       return dom.p(
         {
           className: "font-origin system"
         },
         getStr("fontinspector.system")
       );
@@ -57,22 +84,28 @@ class FontMeta extends PureComponent {
           onClick: this.onCopyURL,
           title: getStr("fontinspector.copyURL"),
         }
       )
     );
   }
 
   renderFontName(name) {
-    return dom.h1(
-      {
-        className: "font-name"
-      },
-      name
-    );
+    if (Services.prefs.getBoolPref(FONT_HIGHLIGHTER_PREF)) {
+      return dom.h1(
+        {
+          className: "font-name",
+          onMouseOver: this.onNameMouseOver,
+          onMouseOut: this.onNameMouseOut,
+        },
+        name
+      );
+    }
+
+    return dom.h1({ className: "font-name" }, name);
   }
 
   render() {
     const {
       name,
       URI,
     } = this.props.font;
 
--- a/devtools/client/inspector/fonts/components/FontOverview.js
+++ b/devtools/client/inspector/fonts/components/FontOverview.js
@@ -15,63 +15,70 @@ const { getStr } = require("../utils/l10
 const Types = require("../types");
 
 class FontOverview extends PureComponent {
   static get propTypes() {
     return {
       fontData: PropTypes.shape(Types.fontData).isRequired,
       fontOptions: PropTypes.shape(Types.fontOptions).isRequired,
       onPreviewFonts: PropTypes.func.isRequired,
+      onToggleFontHighlight: PropTypes.func.isRequired,
     };
   }
 
   renderElementFonts() {
     let {
       fontData,
       fontOptions,
       onPreviewFonts,
+      onToggleFontHighlight,
     } = this.props;
     let { fonts } = fontData;
 
     return fonts.length ?
       FontList({
         fonts,
         fontOptions,
-        onPreviewFonts
+        isCurrentElementFonts: true,
+        onPreviewFonts,
+        onToggleFontHighlight,
       })
       :
       dom.div(
         {
           className: "devtools-sidepanel-no-result"
         },
         getStr("fontinspector.noFontsOnSelectedElement")
       );
   }
 
   renderOtherFonts() {
     let {
       fontData,
+      fontOptions,
       onPreviewFonts,
-      fontOptions,
+      onToggleFontHighlight,
     } = this.props;
     let { otherFonts } = fontData;
 
     if (!otherFonts.length) {
       return null;
     }
 
     return Accordion({
       items: [
         {
           header: getStr("fontinspector.otherFontsInPageHeader"),
           component: FontList,
           componentProps: {
             fontOptions,
             fonts: otherFonts,
-            onPreviewFonts
+            isCurrentElementFonts: false,
+            onPreviewFonts,
+            onToggleFontHighlight,
           },
           opened: false
         }
       ]
     });
   }
 
   render() {
--- a/devtools/client/inspector/fonts/components/FontsApp.js
+++ b/devtools/client/inspector/fonts/components/FontsApp.js
@@ -18,27 +18,29 @@ class FontsApp extends PureComponent {
   static get propTypes() {
     return {
       fontData: PropTypes.shape(Types.fontData).isRequired,
       fontEditor: PropTypes.shape(Types.fontEditor).isRequired,
       fontOptions: PropTypes.shape(Types.fontOptions).isRequired,
       onAxisUpdate: PropTypes.func.isRequired,
       onInstanceChange: PropTypes.func.isRequired,
       onPreviewFonts: PropTypes.func.isRequired,
+      onToggleFontHighlight: PropTypes.func.isRequired,
     };
   }
 
   render() {
     const {
       fontData,
       fontEditor,
       fontOptions,
       onAxisUpdate,
       onInstanceChange,
       onPreviewFonts,
+      onToggleFontHighlight,
     } = this.props;
 
     return dom.div(
       {
         className: "theme-sidebar inspector-tabpanel",
         id: "sidebar-panel-fontinspector"
       },
       fontEditor.isVisible ?
@@ -47,14 +49,15 @@ class FontsApp extends PureComponent {
           onAxisUpdate,
           onInstanceChange,
         })
         :
         FontOverview({
           fontData,
           fontOptions,
           onPreviewFonts,
+          onToggleFontHighlight,
         })
     );
   }
 }
 
 module.exports = connect(state => state)(FontsApp);
--- a/devtools/client/inspector/fonts/fonts.js
+++ b/devtools/client/inspector/fonts/fonts.js
@@ -53,32 +53,34 @@ class FontInspector {
 
     this.snapshotChanges = debounce(this.snapshotChanges, 100, this);
     this.syncChanges = throttle(this.syncChanges, 100, this);
     this.onAxisUpdate = this.onAxisUpdate.bind(this);
     this.onInstanceChange = this.onInstanceChange.bind(this);
     this.onNewNode = this.onNewNode.bind(this);
     this.onPreviewFonts = this.onPreviewFonts.bind(this);
     this.onRuleSelected = this.onRuleSelected.bind(this);
+    this.onToggleFontHighlight = this.onToggleFontHighlight.bind(this);
     this.onRuleUnselected = this.onRuleUnselected.bind(this);
     this.onRuleUpdated = this.onRuleUpdated.bind(this);
     this.onThemeChanged = this.onThemeChanged.bind(this);
     this.update = this.update.bind(this);
 
     this.init();
   }
 
   init() {
     if (!this.inspector) {
       return;
     }
 
     let fontsApp = FontsApp({
       onAxisUpdate: this.onAxisUpdate,
       onInstanceChange: this.onInstanceChange,
+      onToggleFontHighlight: this.onToggleFontHighlight,
       onPreviewFonts: this.onPreviewFonts,
     });
 
     let provider = createElement(Provider, {
       id: "fontinspector",
       key: "fontinspector",
       store: this.store,
       title: INSPECTOR_L10N.getStr("inspector.sidebar.fontInspectorTitle"),
@@ -308,16 +310,59 @@ class FontInspector {
     if (editorId === FONT_EDITOR_ID && rule == this.selectedRule) {
       this.selectedRule = null;
       this.store.dispatch(toggleFontEditor(false));
       this.store.dispatch(resetFontEditor());
       this.ruleView.off("property-value-updated", this.onRuleUpdated);
     }
   }
 
+    /**
+   * Reveal a font's usage in the page.
+   *
+   * @param  {String} font
+   *         The name of the font to be revealed in the page.
+   * @param  {Boolean} show
+   *         Whether or not to reveal the font.
+   * @param  {Boolean} isForCurrentElement
+   *         Whether or not to reveal the font for the current element selection.
+   */
+  async onToggleFontHighlight(font, show, isForCurrentElement) {
+    if (!this.fontsHighlighter) {
+      try {
+        this.fontsHighlighter = await this.inspector.toolbox.highlighterUtils
+                                          .getHighlighterByType("FontsHighlighter");
+      } catch (e) {
+        // When connecting to an older server or when debugging a XUL document, the
+        // FontsHighlighter won't be available. Silently fail here and prevent any future
+        // calls to the function.
+        this.onToggleFontHighlight = () => {};
+        return;
+      }
+    }
+
+    try {
+      if (show) {
+        let node = isForCurrentElement
+                   ? this.inspector.selection.nodeFront
+                   : this.inspector.walker.rootNode;
+
+        await this.fontsHighlighter.show(node, {
+          CSSFamilyName: font.CSSFamilyName,
+          name: font.name,
+        });
+      } else {
+        await this.fontsHighlighter.hide();
+      }
+    } catch (e) {
+      // Silently handle protocol errors here, because these might be called during
+      // shutdown of the browser or devtools, and we don't care if they fail.
+    }
+  }
+
   /**
    * Handler for the "theme-switched" event.
    */
   onThemeChanged(frame) {
     if (frame === this.document.defaultView) {
       this.update();
     }
   }
--- a/devtools/client/inspector/fonts/test/browser.ini
+++ b/devtools/client/inspector/fonts/test/browser.ini
@@ -17,10 +17,11 @@ support-files =
 
 [browser_fontinspector.js]
 [browser_fontinspector_copy-URL.js]
 skip-if = !e10s # too slow on !e10s, logging fully serialized actors (Bug 1446595)
 subsuite = clipboard
 [browser_fontinspector_edit-previews.js]
 [browser_fontinspector_expand-css-code.js]
 [browser_fontinspector_other-fonts.js]
+[browser_fontinspector_reveal-in-page.js]
 [browser_fontinspector_text-node.js]
 [browser_fontinspector_theme-change.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/fonts/test/browser_fontinspector_reveal-in-page.js
@@ -0,0 +1,68 @@
+/* vim: set 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 fonts usage can be revealed in the page using the FontsHighlighter.
+
+const TEST_URI = URL_ROOT + "browser_fontinspector.html";
+
+add_task(async function() {
+  // Enable the feature, since it's off by default for now.
+  await pushPref("devtools.inspector.fonthighlighter.enabled", true);
+
+  // Make sure the toolbox is tall enough to accomodate all fonts, otherwise mouseover
+  // events simulation will fail.
+  await pushPref("devtools.toolbox.footer.height", 500);
+
+  let { tab, view } = await openFontInspectorForURL(TEST_URI);
+  const viewDoc = view.document;
+
+  let fontEls = getUsedFontsEls(viewDoc);
+
+  // The number of window selection change events we expect to get as we hover over each
+  // font in the list. Waiting for those events is how we know that text-runs were
+  // highlighted in the page.
+  // The reason why these numbers vary is because the highlighter may create more than
+  // 1 selection range object, depending on the number of text-runs found.
+  let expectedSelectionChangeEvents = [1, 2, 2, 1, 1];
+
+  for (let i = 0; i < fontEls.length; i++) {
+    info(`Mousing over and out of font number ${i} in the list`);
+
+    // Simulating a mouse over event on the font name and expecting a selectionchange.
+    let nameEl = fontEls[i].querySelector(".font-name");
+    let onEvents = waitForNSelectionEvents(tab, expectedSelectionChangeEvents[i]);
+    EventUtils.synthesizeMouse(nameEl, 2, 2, {type: "mouseover"}, viewDoc.defaultView);
+    await onEvents;
+
+    ok(true,
+      `${expectedSelectionChangeEvents[i]} selectionchange events detected on mouseover`);
+
+    // Simulating a mouse out event on the font name and expecting a selectionchange.
+    let otherEl = fontEls[i].querySelector(".font-preview");
+    onEvents = waitForNSelectionEvents(tab, 1);
+    EventUtils.synthesizeMouse(otherEl, 2, 2, {type: "mouseover"}, viewDoc.defaultView);
+    await onEvents;
+
+    ok(true, "1 selectionchange events detected on mouseout");
+  }
+});
+
+async function waitForNSelectionEvents(tab, numberOfTimes) {
+  await ContentTask.spawn(tab.linkedBrowser, numberOfTimes, async function(n) {
+    let win = content.wrappedJSObject;
+
+    await new Promise(resolve => {
+      let received = 0;
+      win.document.addEventListener("selectionchange", function listen() {
+        received++;
+
+        if (received === n) {
+          win.document.removeEventListener("selectionchange", listen);
+          resolve();
+        }
+      });
+    });
+  });
+}
--- a/devtools/client/inspector/fonts/test/head.js
+++ b/devtools/client/inspector/fonts/test/head.js
@@ -30,25 +30,26 @@ selectNode = async function(node, inspec
 };
 
 /**
  * Adds a new tab with the given URL, opens the inspector and selects the
  * font-inspector tab.
  * @return {Promise} resolves to a {toolbox, inspector, view} object
  */
 var openFontInspectorForURL = async function(url) {
-  await addTab(url);
+  let tab = await addTab(url);
   let {toolbox, inspector} = await openInspector();
 
   // Call selectNode again here to force a fontinspector update since we don't
   // know if the fontinspector-updated event has been sent while the inspector
   // was being opened or not.
   await selectNode("body", inspector);
 
   return {
+    tab,
     toolbox,
     inspector,
     view: inspector.fontinspector
   };
 };
 
 /**
  * Focus one of the preview inputs, clear it, type new text into it and wait for the
--- a/devtools/client/preferences/devtools-client.js
+++ b/devtools/client/preferences/devtools-client.js
@@ -80,16 +80,18 @@ pref("devtools.flexboxinspector.enabled"
 // Enable the new Animation Inspector in Nightly only
 #if defined(NIGHTLY_BUILD)
 pref("devtools.new-animationinspector.enabled", true);
 #else
 pref("devtools.new-animationinspector.enabled", false);
 #endif
 // Enable the Variable Fonts editor
 pref("devtools.inspector.fonteditor.enabled", false);
+// Enable the font highlight-on-hover feature
+pref("devtools.inspector.fonthighlighter.enabled", false);
 
 // Grid highlighter preferences
 pref("devtools.gridinspector.gridOutlineMaxColumns", 50);
 pref("devtools.gridinspector.gridOutlineMaxRows", 50);
 pref("devtools.gridinspector.showGridAreas", false);
 pref("devtools.gridinspector.showGridLineNumbers", false);
 pref("devtools.gridinspector.showInfiniteLines", false);
 
--- a/devtools/server/actors/highlighters.js
+++ b/devtools/server/actors/highlighters.js
@@ -705,14 +705,15 @@ HighlighterEnvironment.prototype = {
   }
 };
 
 register("BoxModelHighlighter", "box-model");
 register("CssGridHighlighter", "css-grid");
 register("CssTransformHighlighter", "css-transform");
 register("EyeDropper", "eye-dropper");
 register("FlexboxHighlighter", "flexbox");
+register("FontsHighlighter", "fonts");
 register("GeometryEditorHighlighter", "geometry-editor");
 register("MeasuringToolHighlighter", "measuring-tool");
 register("PausedDebuggerOverlay", "paused-debugger");
 register("RulersHighlighter", "rulers");
 register("SelectorHighlighter", "selector");
 register("ShapesHighlighter", "shapes");
new file mode 100644
--- /dev/null
+++ b/devtools/server/actors/highlighters/fonts.js
@@ -0,0 +1,88 @@
+ /* 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/. */
+
+"use strict";
+
+const InspectorUtils = require("InspectorUtils");
+
+// How many text runs are we highlighting at a time. There may be many text runs, and we
+// want to prevent performance problems.
+const MAX_TEXT_RANGES = 100;
+
+/**
+ * This highlighter highlights runs of text in the page that have been rendered given a
+ * certain font. The highlighting is done with window selection ranges, so no extra
+ * markup is being inserted into the content page.
+ */
+class FontsHighlighter {
+  constructor(highlighterEnv) {
+    this.env = highlighterEnv;
+  }
+
+  destroy() {
+    this.hide();
+    this.env = this.currentNode = null;
+  }
+
+  get currentNodeDocument() {
+    if (!this.currentNode) {
+      return this.env.document;
+    }
+
+    if (this.currentNode.nodeType === this.currentNode.DOCUMENT_NODE) {
+      return this.currentNode;
+    }
+
+    return this.currentNode.ownerDocument;
+  }
+
+  /**
+   * Show the highlighter for a given node.
+   * @param {DOMNode} node The node in which we want to search for text runs.
+   * @param {Object} options A bunch of options that can be set:
+   * - {String} name The actual font name to look for in the node.
+   * - {String} CSSFamilyName The CSS font-family name given to this font.
+   */
+  show(node, options) {
+    this.currentNode = node;
+    let doc = this.currentNodeDocument;
+
+    // Get all of the fonts used to render content inside the node.
+    let searchRange = doc.createRange();
+    searchRange.selectNodeContents(node);
+
+    let fonts = InspectorUtils.getUsedFontFaces(searchRange, MAX_TEXT_RANGES);
+
+    // Find the ones we want, based on the provided option.
+    let matchingFonts = fonts.filter(f => f.CSSFamilyName === options.CSSFamilyName &&
+                                          f.name === options.name);
+    if (!matchingFonts.length) {
+      return;
+    }
+
+    // Create a multi-selection in the page to highlight the text runs.
+    let selection = doc.defaultView.getSelection();
+    selection.removeAllRanges();
+
+    for (let matchingFont of matchingFonts) {
+      for (let range of matchingFont.ranges) {
+        selection.addRange(range);
+      }
+    }
+  }
+
+  hide() {
+    // No node was highlighted before, don't need to continue any further.
+    if (!this.currentNode) {
+      return;
+    }
+
+    // Simply remove all current ranges in the seletion.
+    let doc = this.currentNodeDocument;
+    let selection = doc.defaultView.getSelection();
+    selection.removeAllRanges();
+  }
+}
+
+exports.FontsHighlighter = FontsHighlighter;
--- a/devtools/server/actors/highlighters/moz.build
+++ b/devtools/server/actors/highlighters/moz.build
@@ -11,16 +11,17 @@ DIRS += [
 DevToolsModules(
     'accessible.js',
     'auto-refresh.js',
     'box-model.js',
     'css-grid.js',
     'css-transform.js',
     'eye-dropper.js',
     'flexbox.js',
+    'fonts.js',
     'geometry-editor.js',
     'measuring-tool.js',
     'paused-debugger.js',
     'rulers.js',
     'selector.js',
     'shapes.js',
     'simple-outline.js',
     'xul-accessible.js'