Bug 1483929 - (Part 3) Revert to showing all used fontsfonts used on the selected element. r=gl
authorRazvan Caliman <rcaliman@mozilla.com>
Tue, 21 Aug 2018 18:20:45 +0000
changeset 488245 ab010295820f0f4b71d6c3d081dcd7da0c4e2f07
parent 488244 9b918e1a1de5238d0f3b82f9d3a2dde4cbe2ffba
child 488246 ec5152902eb717a568b34accf03f2a520862c512
push id9719
push userffxbld-merge
push dateFri, 24 Aug 2018 17:49:46 +0000
treeherdermozilla-beta@719ec98fba77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgl
bugs1483929
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 1483929 - (Part 3) Revert to showing all used fontsfonts used on the selected element. r=gl Removes logic pertaining to filtering by members of CSS font-family declaration. Differential Revision: https://phabricator.services.mozilla.com/D3805
devtools/client/inspector/fonts/actions/font-editor.js
devtools/client/inspector/fonts/components/FontEditor.js
devtools/client/inspector/fonts/fonts.js
devtools/client/inspector/fonts/reducers/font-editor.js
devtools/client/inspector/fonts/test/browser.ini
devtools/client/inspector/fonts/test/browser_fontinspector_family-unused.js
devtools/client/inspector/fonts/types.js
devtools/client/locales/en-US/font-inspector.properties
devtools/client/themes/fonts.css
--- a/devtools/client/inspector/fonts/actions/font-editor.js
+++ b/devtools/client/inspector/fonts/actions/font-editor.js
@@ -39,22 +39,20 @@ module.exports = {
   updateAxis(axis, value) {
     return {
       type: UPDATE_AXIS_VALUE,
       axis,
       value,
     };
   },
 
-  updateFontEditor(fonts, families = { used: [], notUsed: [] }, properties = {},
-                   id = "") {
+  updateFontEditor(fonts, properties = {}, id = "") {
     return {
       type: UPDATE_EDITOR_STATE,
       fonts,
-      families,
       properties,
       id,
     };
   },
 
   updateFontProperty(property, value) {
     return {
       type: UPDATE_PROPERTY_VALUE,
--- a/devtools/client/inspector/fonts/components/FontEditor.js
+++ b/devtools/client/inspector/fonts/components/FontEditor.js
@@ -88,50 +88,24 @@ class FontEditor extends PureComponent {
         onChange: this.props.onPropertyChange,
         step: this.getAxisStep(axis.minValue, axis.maxValue),
         unit: null,
         value: editedAxes[axis.tag] || axis.defaultValue,
       });
     });
   }
 
-  renderFamilesNotUsed(familiesNotUsed = []) {
-    if (!familiesNotUsed.length) {
-      return null;
-    }
-
-    const familiesList = familiesNotUsed.map(family => {
-      return dom.div(
-        {
-          className: "font-family-unused",
-        },
-        family
-      );
-    });
-
-    return dom.details(
-      {},
-      dom.summary(
-        {},
-        getStr("fontinspector.familiesUnusedLabel")
-      ),
-      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.
    * @return {DOMNode}
    */
-  renderFontFamily(fonts, families) {
+  renderFontFamily(fonts) {
     if (!fonts.length) {
       return null;
     }
 
     const topUsedFontsList = this.renderFontList(fonts.slice(0, MAX_FONTS));
     const moreUsedFontsList = this.renderFontList(fonts.slice(MAX_FONTS, fonts.length));
     const moreUsedFonts = moreUsedFontsList === null
       ? null
@@ -153,18 +127,17 @@ class FontEditor extends PureComponent {
         },
         getStr("fontinspector.fontFamilyLabel")
       ),
       dom.div(
         {
           className: "font-control-box",
         },
         topUsedFontsList,
-        moreUsedFonts,
-        this.renderFamilesNotUsed(families.notUsed)
+        moreUsedFonts
       )
     );
   }
 
   /**
    * Given an array of fonts, get an unordered list with rendered FontMeta components.
    * If the array of fonts is empty, return null.
    *
@@ -294,17 +267,17 @@ class FontEditor extends PureComponent {
         },
         warning
       )
     );
   }
 
   render() {
     const { fontEditor } = this.props;
-    const { fonts, families, axes, instance, properties, warning } = fontEditor;
+    const { fonts, axes, instance, properties, warning } = 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";
     });
@@ -317,17 +290,17 @@ class FontEditor extends PureComponent {
       return this.renderWarning(warning);
     }
 
     return dom.div(
       {
         id: "font-editor"
       },
       // Always render UI for font family, format and font file URL.
-      this.renderFontFamily(fonts, families),
+      this.renderFontFamily(fonts),
       // 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"]),
       // Always render UI for line height.
       this.renderLineHeight(properties["line-height"]),
       // Render UI for font weight if no "wght" registered axis is defined.
       !hasWeightAxis && this.renderFontWeight(properties["font-weight"]),
--- a/devtools/client/inspector/fonts/fonts.js
+++ b/devtools/client/inspector/fonts/fonts.js
@@ -301,61 +301,16 @@ class FontInspector {
     this.ruleView = null;
     this.selectedRule = null;
     this.store = null;
     this.writers.clear();
     this.writers = null;
   }
 
   /**
-   * Get a subset of fonts used on a node whose font family names are found in the
-   * node's CSS font-family property value. The fonts will be sorted in the order their
-   * family names are declared in CSS font-family.
-   *
-   * Fonts returned by this.getFontsForNode() contain, among others, these attributes:
-   * - CSSFamilyName: a string of the font's family name (ex: "Times");
-   * - CSSGeneric: a string of the generic font family (ex: "serif", "sans-serif") if
-   * the font was resolved from a generic font family keyword, like serif, instead of
-   * an explicit font famly, like "Times". If the font is resolved from an
-   * explicit font family, CSSGeneric is null.
-   *
-   * For example:
-   * font-family: "Avenir", serif;
-   *
-   * If fonts from both families are used, it will yield:
-   * { CSSFamilyName: "Avenir", CSSGeneric: null, ... },
-   * { CSSFamilyName: "Times", CSSGeneric: "serif", ... },
-   *
-   * @param {Array} fonts
-   *        Fonts used on a node got from a call to this.getFontsForNode().
-   * @param {Array} fontFamilies
-   *        Strings of font families from a node's CSS font-family property value.
-   * @return {Array}
-   *         Subset of `fonts` whose font family names appear in `fontFamilies`.
-   */
-  filterFontsUsed(fonts = [], fontFamilies = []) {
-    return fontFamilies.reduce((acc, family) => {
-      const match = fonts.find(font => {
-        const generic = typeof font.CSSGeneric === "string"
-          ? font.CSSGeneric.toLowerCase()
-          : font.CSSGeneric;
-
-        return generic === family.toLowerCase()
-          || font.CSSFamilyName.toLowerCase() === family.toLowerCase();
-      });
-
-      if (match) {
-        acc.push(match);
-      }
-
-      return acc;
-    }, []);
-  }
-
-  /**
    * Get all expected CSS font properties and values from the node's matching rules and
    * fallback to computed style. Skip CSS Custom Properties, `calc()` and keyword values.
    *
    * @return {Object}
    */
   async getFontProperties() {
     const properties = {};
 
@@ -571,43 +526,16 @@ class FontInspector {
     } else {
       this.writers.set(name, this.updateFontVariationSettings);
     }
 
     return this.writers.get(name);
   }
 
   /**
-   * Given a list of font families, return an object that groups them into sets of used
-   * and not used if they match families of fonts from the given list of fonts used on a
-   * node.
-   *
-   * @See this.filterFontsUsed() for an explanation of CSSFamilyName and CSSGeneric.
-   *
-   * @param {Array} fontsUsed
-   *        Fonts used on a node.
-   * @param {Array} fontFamilies
-   *        Strings of font families
-   * @return {Object}
-   */
-  groupFontFamilies(fontsUsed = [], fontFamilies = []) {
-    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
-      .filter(family => !familiesUsedLowercase.includes(family.toLowerCase()));
-
-    return families;
-  }
-
-  /**
    * Check if the font inspector panel is visible.
    *
    * @return {Boolean}
    */
   isPanelVisible() {
     return this.inspector &&
            this.inspector.sidebar &&
            this.inspector.sidebar.getCurrentTabID() === "fontinspector";
@@ -938,41 +866,23 @@ class FontInspector {
       await this.ruleView.selectElement(node, false);
     }
 
     // Select the node's inline style as the rule where to write property value changes.
     this.selectedRule =
       this.ruleView.rules.find(rule => rule.domRule.type === ELEMENT_STYLE);
 
     const properties = await this.getFontProperties();
-    const familiesDeclared =
-      properties["font-family"].split(",")
-      .map(font => font.replace(/["']+/g, "").trim());
-    // Subset of fonts used on the node whose family names exist in CSS font-family.
-    let fontsUsed = this.filterFontsUsed(fonts, familiesDeclared);
-    // Object with font families groupped by used and not used.
-    const families = this.groupFontFamilies(fontsUsed, familiesDeclared);
     // Assign writer methods to each axis defined in font-variation-settings.
     const axes = parseFontVariationAxes(properties["font-variation-settings"]);
     Object.keys(axes).map(axis => {
       this.writers.set(axis, this.getWriterForAxis(axis));
     });
 
-    // Pick fonts from descendants if no declared fonts were used on this node.
-    if (!fontsUsed.length && fonts.length) {
-      const otherVarFonts = fonts.filter(font => {
-        return (font.variationAxes && font.variationAxes.length);
-      });
-
-      // Prefer picking variable fonts if any were found on descendants of this node.
-      // The FontEditor component will render UI for the first font in the list.
-      fontsUsed = otherVarFonts.length ? otherVarFonts : fonts;
-    }
-
-    this.store.dispatch(updateFontEditor(fontsUsed, families, properties, node.actorID));
+    this.store.dispatch(updateFontEditor(fonts, properties, node.actorID));
     this.inspector.emit("fonteditor-updated");
     // Listen to manual changes in the Rule view that could update the Font Editor state
     this.ruleView.on("property-value-updated", this.onRulePropertyUpdated);
   }
 
   /**
    * Capture the state of all variation axes. Allows the user to return to this state with
    * the "Custom" instance after they've selected a font-defined named variation instance.
--- a/devtools/client/inspector/fonts/reducers/font-editor.js
+++ b/devtools/client/inspector/fonts/reducers/font-editor.js
@@ -17,25 +17,17 @@ const {
   UPDATE_WARNING_MESSAGE,
 } = require("../actions/index");
 
 const INITIAL_STATE = {
   // Variable font axes.
   axes: {},
   // Copy of the most recent axes values. Used to revert from a named instance.
   customInstanceValues: [],
-  // Font families declared on the selected element
-  families: {
-    // Names of font families used
-    used: [],
-    // Names of font families declared but not used
-    notUsed: []
-  },
-  // Fonts whose family names are declared in CSS font-family and used
-  // on the selected element.
+  // Fonts used on the selected element.
   fonts: [],
   // Current selected font variation instance.
   instance: {
     name: getStr("fontinspector.customInstanceName"),
     values: [],
   },
   // CSS font properties defined on the selected rule.
   properties: {},
@@ -78,17 +70,17 @@ const reducers = {
   [UPDATE_CUSTOM_INSTANCE](state) {
     const newState = { ...state };
     newState.customInstanceValues = Object.keys(state.axes).map(axis => {
       return { axis: [axis], value: state.axes[axis] };
     });
     return newState;
   },
 
-  [UPDATE_EDITOR_STATE](state, { fonts, families, properties, id }) {
+  [UPDATE_EDITOR_STATE](state, { fonts, properties, id }) {
     const axes = parseFontVariationAxes(properties["font-variation-settings"]);
 
     // If not defined in font-variation-settings, setup "wght" axis with the value of
     // "font-weight" if it is numeric and not a keyword.
     const weight = properties["font-weight"];
     if (axes.wght === undefined && parseFloat(weight).toString() === weight.toString()) {
       axes.wght = weight;
     }
@@ -98,17 +90,17 @@ const reducers = {
     const stretch = properties["font-stretch"];
     // Match the number part from values like: 10%, 10.55%, 0.2%
     // If there's a match, the number is the second item in the match array.
     const match = stretch.trim().match(/^(\d+(.\d+)?)%$/);
     if (axes.wdth === undefined && match && match[1]) {
       axes.wdth = match[1];
     }
 
-    return { ...state, axes, fonts, families, properties, id };
+    return { ...state, axes, fonts, properties, id };
   },
 
   [UPDATE_PROPERTY_VALUE](state, { property, value }) {
     const newState = { ...state };
     newState.properties[property] = value;
     return newState;
   },
 
--- a/devtools/client/inspector/fonts/test/browser.ini
+++ b/devtools/client/inspector/fonts/test/browser.ini
@@ -20,14 +20,13 @@ support-files =
 skip-if = !e10s # too slow on !e10s, logging fully serialized actors (Bug 1446595)
 subsuite = clipboard
 [browser_fontinspector_all-fonts.js]
 [browser_fontinspector_edit-previews.js]
 [browser_fontinspector_editor-font-size-conversion.js]
 [browser_fontinspector_editor-values.js]
 [browser_fontinspector_editor-keywords.js]
 [browser_fontinspector_expand-css-code.js]
-[browser_fontinspector_family-unused.js]
 [browser_fontinspector_no-fonts.js]
 [browser_fontinspector_other-fonts.js]
 [browser_fontinspector_reveal-in-page.js]
 [browser_fontinspector_text-node.js]
 [browser_fontinspector_theme-change.js]
deleted file mode 100644
--- a/devtools/client/inspector/fonts/test/browser_fontinspector_family-unused.js
+++ /dev/null
@@ -1,39 +0,0 @@
-/* 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 + "doc_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/inspector/fonts/types.js
+++ b/devtools/client/inspector/fonts/types.js
@@ -77,36 +77,25 @@ const font = exports.font = {
   variationInstances: PropTypes.arrayOf(PropTypes.shape(fontVariationInstance))
 };
 
 exports.fontOptions = {
   // The current preview text
   previewText: PropTypes.string,
 };
 
-const fontFamilies = {
-  // Font family names used on the selected element
-  used: PropTypes.arrayOf(PropTypes.string),
-
-  // Font family names declared but not used on the selected element
-  notUsed: PropTypes.arrayOf(PropTypes.string),
-};
-
 exports.fontEditor = {
   // Variable font axes and their values
   axes: PropTypes.object,
 
   // Axes values changed at runtime structured like the "values" property
   // of a fontVariationInstance
   customInstanceValues: PropTypes.array,
 
-  // Font families declared on this element
-  families: PropTypes.shape(fontFamilies),
-
-  // Fonts used on the selected element whose family names are declared in CSS font-family
+  // Fonts used on the selected element
   fonts: PropTypes.arrayOf(PropTypes.shape(font)),
 
   // Font variation instance currently selected
   instance: PropTypes.shape(fontVariationInstance),
 
   // CSS font properties defined on the element
   properties: PropTypes.object,
 };
--- a/devtools/client/locales/en-US/font-inspector.properties
+++ b/devtools/client/locales/en-US/font-inspector.properties
@@ -51,20 +51,16 @@ 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.familiesUnusedLabel): Label for the list which
-# contains the names of font families declared but not used.
-fontinspector.familiesUnusedLabel=Unused font families
-
 # LOCALIZATION NOTE (fontinspector.showMore): Label for a collapsed list of fonts.
 fontinspector.showMore=Show more
 
 # LOCALIZATION NOTE (fontinspector.showLess): Label for an expanded list of fonts.
 fontinspector.showLess=Show less
 
 # LOCALIZATION NOTE (fontinspector.noPseduoWarning): Warning message shown when the
 # selected element is a pseudo-element that the font editor cannot work with.
--- a/devtools/client/themes/fonts.css
+++ b/devtools/client/themes/fonts.css
@@ -216,22 +216,16 @@
   display: inline-block;
   flex: 1;
   font-size: 12px;
   max-width: 70px;
   margin-right: 10px;
   -moz-user-select: none;
 }
 
-.font-family-unused {
-  margin-bottom: .3em;
-  margin-left: 1.15em;
-  color: var(--grey-50);
-}
-
 .font-value-input {
   margin-left: 10px;
   text-align: right;
   width: 60px;
   padding: 2px 3px;
 }
 
 .font-value-input,