Bug 1465500 - Show list of font families declared but not used. r=gl draft
authorRazvan Caliman <rcaliman@mozilla.com>
Wed, 06 Jun 2018 14:06:47 +0200
changeset 811214 f2a5d603cacc5c75f655e4d3fb8f99976add60ee
parent 811103 1c235a552c32ba6c97e6030c497c49f72c7d48a8
push id114240
push userbmo:rcaliman@mozilla.com
push dateWed, 27 Jun 2018 08:37:06 +0000
reviewersgl
bugs1465500
milestone63.0a1
Bug 1465500 - Show list of font families declared but not used. r=gl MozReview-Commit-ID: 4GfpV8RmK0N
devtools/client/inspector/fonts/components/FontEditor.js
devtools/client/inspector/fonts/fonts.js
devtools/client/inspector/fonts/test/browser.ini
devtools/client/inspector/fonts/test/browser_fontinspector.html
devtools/client/inspector/fonts/test/browser_fontinspector_family-unused.js
devtools/client/locales/en-US/font-inspector.properties
devtools/client/themes/fonts.css
--- a/devtools/client/inspector/fonts/components/FontEditor.js
+++ b/devtools/client/inspector/fonts/components/FontEditor.js
@@ -1,16 +1,17 @@
 /* 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 { createFactory, PureComponent } = require("devtools/client/shared/vendor/react");
 const dom = require("devtools/client/shared/vendor/react-dom-factories");
+const { PluralForm } = require("devtools/shared/plural-form");
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
 
 const FontMeta = createFactory(require("./FontMeta"));
 const FontPropertyValue = createFactory(require("./FontPropertyValue"));
 const FontSize = createFactory(require("./FontSize"));
 const FontStyle = createFactory(require("./FontStyle"));
 const FontWeight = createFactory(require("./FontWeight"));
 
@@ -74,26 +75,59 @@ class FontEditor extends PureComponent {
         step: this.getAxisStep(axis.minValue, axis.maxValue),
         label: axis.name,
         name: axis.tag,
         onChange: this.props.onPropertyChange,
         unit: null
       });
     });
   }
+
+  renderFamilesNotUsed(familiesNotUsed = []) {
+    if (!familiesNotUsed.length) {
+      return null;
+    }
+
+    const familiesNotUsedLabel = PluralForm
+      .get(familiesNotUsed.length, getStr("fontinspector.familiesNotUsedLabel"))
+      .replace("#1", familiesNotUsed.length);
+
+    const familiesList = familiesNotUsed.map(family => {
+      return dom.div(
+        {
+          className: "font-family-unused",
+        },
+        family
+      );
+    });
+
+    return dom.details(
+      {},
+      dom.summary(
+        {
+          className: "font-family-unused-header",
+        },
+        familiesNotUsedLabel
+      ),
+      familiesList
+    );
+  }
+
   /**
    * Render font family, font name, and metadata for all fonts used on selected node.
    *
    * @param {Array} fonts
    *        Fonts used on selected node.
+   * @param {Array} families
+   *        Font familes declared on selected node.
    * @param {Function} onToggleFontHighlight
    *        Callback to trigger in-context highlighting of text that uses a font.
    * @return {DOMNode}
    */
-  renderFontFamily(fonts, onToggleFontHighlight) {
+  renderFontFamily(fonts, families, onToggleFontHighlight) {
     if (!fonts.length) {
       return null;
     }
 
     const fontList = dom.ul(
       {
         className: "fonts-list"
       },
@@ -114,17 +148,18 @@ class FontEditor extends PureComponent {
           className: "font-control-label",
         },
         getStr("fontinspector.fontFamilyLabel")
       ),
       dom.div(
         {
           className: "font-control-box",
         },
-        fontList
+        fontList,
+        this.renderFamilesNotUsed(families.notUsed)
       )
     );
   }
 
   renderFontSize(value) {
     return value && FontSize({
       onChange: this.props.onPropertyChange,
       value,
@@ -210,17 +245,17 @@ class FontEditor extends PureComponent {
         className: "devtools-sidepanel-no-result"
       },
       getStr("fontinspector.noFontsOnSelectedElement")
     );
   }
 
   render() {
     const { fontEditor, onToggleFontHighlight } = this.props;
-    const { fonts, axes, instance, properties } = fontEditor;
+    const { fonts, families, axes, instance, properties } = fontEditor;
     // Pick the first font to show editor controls regardless of how many fonts are used.
     const font = fonts[0];
     const hasFontAxes = font && font.variationAxes;
     const hasFontInstances = font && font.variationInstances
       && font.variationInstances.length > 0;
     const hasSlantOrItalicAxis = hasFontAxes && font.variationAxes.find(axis => {
       return axis.tag === "slnt" || axis.tag === "ital";
     });
@@ -232,17 +267,17 @@ class FontEditor extends PureComponent {
 
     return dom.div(
       {
         id: "font-editor"
       },
       // Render empty state message for nodes that don't have font properties.
       !hasWeight && this.renderWarning(),
       // Always render UI for font family, format and font file URL.
-      this.renderFontFamily(fonts, onToggleFontHighlight),
+      this.renderFontFamily(fonts, families, onToggleFontHighlight),
       // Render UI for font variation instances if they are defined.
       hasFontInstances && this.renderInstances(font.variationInstances, instance),
       // Always render UI for font size.
       this.renderFontSize(properties["font-size"]),
       // Render UI for font weight if no "wght" registered axis is defined.
       !hasWeightAxis && this.renderFontWeight(properties["font-weight"]),
       // Render UI for font style if no "slnt" or "ital" registered axis is defined.
       !hasSlantOrItalicAxis && this.renderFontStyle(properties["font-style"]),
--- a/devtools/client/inspector/fonts/fonts.js
+++ b/devtools/client/inspector/fonts/fonts.js
@@ -410,18 +410,17 @@ class FontInspector {
     const families = {};
     // Font family names declared and used.
     families.used = fontsUsed.map(font =>
       font.CSSGeneric ? font.CSSGeneric : font.CSSFamilyName
     );
     const familiesUsedLowercase = families.used.map(family => family.toLowerCase());
     // Font family names declared but not used.
     families.notUsed = fontFamilies
-      .map(family => family.toLowerCase())
-      .filter(family => !familiesUsedLowercase.includes(family));
+      .filter(family => !familiesUsedLowercase.includes(family.toLowerCase()));
 
     return families;
   }
 
   /**
    * Check if the font inspector panel is visible.
    *
    * @return {Boolean}
--- a/devtools/client/inspector/fonts/test/browser.ini
+++ b/devtools/client/inspector/fonts/test/browser.ini
@@ -17,11 +17,12 @@ 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_editor-values.js]
 [browser_fontinspector_expand-css-code.js]
+[browser_fontinspector_family-unused.js]
 [browser_fontinspector_other-fonts.js]
 [browser_fontinspector_reveal-in-page.js]
 [browser_fontinspector_theme-change.js]
--- a/devtools/client/inspector/fonts/test/browser_fontinspector.html
+++ b/devtools/client/inspector/fonts/test/browser_fontinspector.html
@@ -21,19 +21,18 @@
     src: url(ostrich-black.ttf);
   }
   body{
     /* Arial doesn't exist on Linux. Liberation Sans is the default sans-serif there. */
     font-family:Arial, "Liberation Sans";
     font-size: 36px;
   }
   div {
-    font-family:Arial;
-    font-family:bar;
     font-size: 1em;
+    font-family:bar, "Missing Family", sans-serif;
   }
   .normal-text {
     font-family: barnormal;
     font-weight: normal;
   }
   .bold-text {
     font-family: bar;
     font-weight: bold;
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/fonts/test/browser_fontinspector_family-unused.js
@@ -0,0 +1,39 @@
+/* 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";
+
+const TEST_URI = URL_ROOT + "browser_fontinspector.html";
+
+// Test that unused font families show up in the font editor.
+add_task(async function() {
+  await pushPref("devtools.inspector.fonteditor.enabled", true);
+  const { inspector, view } = await openFontInspectorForURL(TEST_URI);
+  const viewDoc = view.document;
+
+  await testFamiliesUnused(inspector, viewDoc);
+  await testZeroFamiliesUnused(inspector, viewDoc);
+});
+
+function getUnusedFontFamilies(viewDoc) {
+  return [...viewDoc.querySelectorAll("#font-editor .font-family-unused")]
+    .map(el => el.textContent);
+}
+
+async function testFamiliesUnused(inspector, viewDoc) {
+  await selectNode("div", inspector);
+
+  const unused = getUnusedFontFamilies(viewDoc);
+  is(unused.length, 2, "Two font families were not used");
+  is(unused[0], "Missing Family", "First unused family is correct");
+  is(unused[1], "sans-serif", "Second unused family is correct");
+}
+
+async function testZeroFamiliesUnused(inspector, viewDoc) {
+  await selectNode(".normal-text", inspector);
+
+  const unused = getUnusedFontFamilies(viewDoc);
+  const header = viewDoc.querySelector("#font-editor .font-family-unused-header");
+  is(unused.length, 0, "All font families were used");
+  is(header, null, "Container for unused font families was not rendered");
+}
--- a/devtools/client/locales/en-US/font-inspector.properties
+++ b/devtools/client/locales/en-US/font-inspector.properties
@@ -50,8 +50,14 @@ fontinspector.fontSizeLabel=Size
 
 # LOCALIZATION NOTE (fontinspector.fontWeightLabel): This label is shown next to the UI
 # in the font editor which allows the user to change the font weight.
 fontinspector.fontWeightLabel=Weight
 
 # LOCALIZATION NOTE (fontinspector.fontItalicLabel): This label is shown next to the UI
 # in the font editor which allows the user to change the style of the font to italic.
 fontinspector.fontItalicLabel=Italic
+
+# # LOCALIZATION NOTE (fontinspector.familiesNotUsedLabel): Semi-colon list of
+# plural forms.
+# See: http://developer.mozilla.org/en/docs/Localization_and_Plurals
+# #1 is the number of unused font families
+fontinspector.familiesNotUsedLabel=1 not used;#1 not used
--- a/devtools/client/themes/fonts.css
+++ b/devtools/client/themes/fonts.css
@@ -151,16 +151,27 @@
   display: inline-block;
   flex: 1;
   font-size: 12px;
   min-width: 80px;
   margin-right: 10px;
   -moz-user-select: none;
 }
 
+.font-family-unused-header {
+  -moz-user-select: none;
+  margin-bottom: .7em;
+  cursor: pointer;
+}
+
+.font-family-unused {
+  margin-bottom: .3em;
+  color: var(--grey-50);
+}
+
 .font-instance-select:active{
   outline: none;
 }
 
 .font-value-input {
   margin-left: 10px;
   width: 60px;
 }