Bug 1485324 - (Part 3) Remove obsolete font on element no longer used in Fonts panel; r=gl
authorRazvan Caliman <rcaliman@mozilla.com>
Wed, 14 Nov 2018 16:33:09 +0000
changeset 446565 003af65241156a247b0cfeb4604d1483036a002f
parent 446564 13a15cc93107cbe2a17a332ba364f79c4e04c5ce
child 446566 9c7f5d38a1f4c3d1ca7ddcf74c161206c5c59c82
push id35043
push userebalazs@mozilla.com
push dateThu, 15 Nov 2018 16:12:36 +0000
treeherdermozilla-central@59026ada59bd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgl
bugs1485324
milestone65.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 1485324 - (Part 3) Remove obsolete font on element no longer used in Fonts panel; r=gl Depends on D11506 Removes the `fonts` field from the Redux slice previously used by the FontOverview component in the Font Inspector version of the Fonts panel. This field is duplicated with the `fonts` used by the Font Editor. The telemetry can use that one instead. Refactors the update() method which updates the Redux slice for the FontOverview component. In time, the update() and refreshFontEditor() method will merge. So will the Redux slices: `fontEditor` and `fonts` in order to save a duplicate call to the server for `allFonts`. Differential Revision: https://phabricator.services.mozilla.com/D11507
devtools/client/inspector/fonts/actions/fonts.js
devtools/client/inspector/fonts/fonts.js
devtools/client/inspector/fonts/reducers/fonts.js
devtools/client/inspector/fonts/types.js
--- a/devtools/client/inspector/fonts/actions/fonts.js
+++ b/devtools/client/inspector/fonts/actions/fonts.js
@@ -8,17 +8,16 @@ const {
   UPDATE_FONTS,
 } = require("./index");
 
 module.exports = {
 
   /**
    * Update the list of fonts in the font inspector
    */
-  updateFonts(fonts, allFonts) {
+  updateFonts(allFonts) {
     return {
       type: UPDATE_FONTS,
-      fonts,
       allFonts,
     };
   },
 
 };
--- a/devtools/client/inspector/fonts/fonts.js
+++ b/devtools/client/inspector/fonts/fonts.js
@@ -554,21 +554,21 @@ class FontInspector {
            this.inspector.selection.isElementNode() &&
            !this.inspector.selection.isPseudoElementNode();
   }
 
   /**
    * Upon a new node selection, log some interesting telemetry probes.
    */
   logTelemetryProbesOnNewNode() {
-    const { fontData, fontEditor } = this.store.getState();
+    const { fontEditor } = this.store.getState();
     const { telemetry } = this.inspector;
 
     // Log the number of font faces used to render content of the element.
-    const nbOfFontsRendered = fontData.fonts.length;
+    const nbOfFontsRendered = fontEditor.fonts.length;
     if (nbOfFontsRendered) {
       telemetry.getHistogramById(HISTOGRAM_N_FONTS_RENDERED).add(nbOfFontsRendered);
     }
 
     // Log data about the currently edited font (if any).
     // Note that the edited font is always the first one from the fontEditor.fonts array.
     const editedFont = fontEditor.fonts[0];
     if (!editedFont) {
@@ -892,61 +892,45 @@ class FontInspector {
 
   async update() {
     // Stop refreshing if the inspector or store is already destroyed.
     if (!this.inspector || !this.store) {
       return;
     }
 
     let allFonts = [];
-    let fonts = [];
 
     if (!this.isSelectedNodeValid()) {
-      this.store.dispatch(updateFonts(fonts, allFonts));
+      this.store.dispatch(updateFonts(allFonts));
       return;
     }
 
     const { fontOptions } = this.store.getState();
     const { previewText } = fontOptions;
 
     const options = {
       includePreviews: true,
+      // Coerce the type of `supportsFontVariations` to a boolean.
+      includeVariations: !!this.pageStyle.supportsFontVariations,
       previewText,
       previewFillStyle: getColor("body-color"),
     };
 
-    // Add the includeVariations argument into the option to get font variation data.
-    if (this.pageStyle.supportsFontVariations) {
-      options.includeVariations = true;
-    }
-
-    const node = this.inspector.selection.nodeFront;
-    fonts = await this.getFontsForNode(node, options);
+    // If there are no fonts used on the page, the result is an empty array.
     allFonts = await this.getAllFonts(options);
 
-    if (!fonts.length) {
-      // No fonts to display. Clear the previously shown fonts.
-      if (this.store) {
-        this.store.dispatch(updateFonts(fonts, allFonts));
-      }
-      return;
-    }
-
+    // Augment each font object with a dataURI for an image with a sample of the font.
     for (const font of [...allFonts]) {
       font.previewUrl = await font.preview.data.string();
     }
 
-    // in case we've been destroyed in the meantime
-    if (!this.document) {
-      return;
-    }
-
-    this.store.dispatch(updateFonts(fonts, allFonts));
-
-    this.inspector.emit("fontinspector-updated");
+    // Dispatch to the store if it hasn't been destroyed in the meantime.
+    this.store && this.store.dispatch(updateFonts(allFonts));
+    // Emit on the inspector if it hasn't been destroyed in the meantime.
+    this.inspector && this.inspector.emit("fontinspector-updated");
   }
 
   /**
    * Update the "font-variation-settings" CSS property with the state of all touched
    * font variation axes which shouldn't be written to other CSS font properties.
    */
   updateFontVariationSettings() {
     const fontEditor = this.store.getState().fontEditor;
--- a/devtools/client/inspector/fonts/reducers/fonts.js
+++ b/devtools/client/inspector/fonts/reducers/fonts.js
@@ -6,24 +6,22 @@
 
 const {
   UPDATE_FONTS,
 } = require("../actions/index");
 
 const INITIAL_FONT_DATA = {
   // All fonts on the current page.
   allFonts: [],
-  // Fonts used on the selected element.
-  fonts: [],
 };
 
 const reducers = {
 
-  [UPDATE_FONTS](_, { fonts, allFonts }) {
-    return { fonts, allFonts };
+  [UPDATE_FONTS](_, { allFonts }) {
+    return { allFonts };
   },
 
 };
 
 module.exports = function(fontData = INITIAL_FONT_DATA, action) {
   const reducer = reducers[action.type];
   if (!reducer) {
     return fontData;
--- a/devtools/client/inspector/fonts/types.js
+++ b/devtools/client/inspector/fonts/types.js
@@ -101,12 +101,9 @@ exports.fontEditor = {
 };
 
 /**
  * Font data.
  */
 exports.fontData = {
   // All fonts on the current page.
   allFonts: PropTypes.arrayOf(PropTypes.shape(font)),
-
-  // The fonts used in the current element.
-  fonts: PropTypes.arrayOf(PropTypes.shape(font)),
 };