Bug 1465500 - Show list of font families declared but not used. r=gl
authorRazvan Caliman <rcaliman@mozilla.com>
Wed, 06 Jun 2018 14:06:47 +0200
changeset 423894 c69831eb0e71d8486403caa67d91082a9b821d67
parent 423893 79aabc0f2db2fa8e6c248502aa901efb4ffafe44
child 423895 510771e64350192e232abe0c5b87c4df9f42247d
push id34195
push usertoros@mozilla.com
push dateWed, 27 Jun 2018 22:05:29 +0000
treeherdermozilla-central@e922b59832f1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgl
bugs1465500
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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;
 }