Bug 1464796 - (Part 2) Select declared and used fonts. Group font family names by used and not used. r=pbro
MozReview-Commit-ID: 44RuWZBfzGX
--- a/devtools/client/inspector/fonts/actions/font-editor.js
+++ b/devtools/client/inspector/fonts/actions/font-editor.js
@@ -38,20 +38,21 @@ module.exports = {
updateAxis(axis, value) {
return {
type: UPDATE_AXIS_VALUE,
axis,
value,
};
},
- updateFontEditor(fonts, properties = {}) {
+ updateFontEditor(fonts, families = { used: [], notUsed: [] }, properties = {}) {
return {
type: UPDATE_EDITOR_STATE,
fonts,
+ families,
properties,
};
},
updateFontProperty(property, value) {
return {
type: UPDATE_PROPERTY_VALUE,
property,
--- a/devtools/client/inspector/fonts/components/FontEditor.js
+++ b/devtools/client/inspector/fonts/components/FontEditor.js
@@ -178,22 +178,21 @@ class FontEditor extends PureComponent {
),
instanceSelect
);
}
render() {
const { fontEditor, onToggleFontHighlight } = this.props;
const { fonts, axes, instance, properties } = fontEditor;
- const usedFonts = fonts.filter(font => font.used);
- // If no used fonts were found, pick the first available font.
- // Else, pick the first used font regardless of how many there are.
- const font = usedFonts.length === 0 ? fonts[0] : usedFonts[0];
+ // 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.length > 0;
+ const hasFontInstances = font && font.variationInstances
+ && font.variationInstances.length > 0;
const hasSlantOrItalicAxis = hasFontAxes && font.variationAxes.find(axis => {
return axis.tag === "slnt" || axis.tag === "ital";
});
const hasWeightAxis = hasFontAxes && font.variationAxes.find(axis => {
return axis.tag === "wght";
});
return dom.div(
--- a/devtools/client/inspector/fonts/fonts.js
+++ b/devtools/client/inspector/fonts/fonts.js
@@ -69,18 +69,18 @@ class FontInspector {
this.writers = new Map();
this.snapshotChanges = debounce(this.snapshotChanges, 100, this);
this.syncChanges = debounce(this.syncChanges, 100, this);
this.onInstanceChange = this.onInstanceChange.bind(this);
this.onNewNode = this.onNewNode.bind(this);
this.onPreviewFonts = this.onPreviewFonts.bind(this);
this.onPropertyChange = this.onPropertyChange.bind(this);
+ this.onRuleUpdated = this.onRuleUpdated.bind(this);
this.onToggleFontHighlight = this.onToggleFontHighlight.bind(this);
- this.onRuleUpdated = this.onRuleUpdated.bind(this);
this.onThemeChanged = this.onThemeChanged.bind(this);
this.update = this.update.bind(this);
this.updateFontVariationSettings = this.updateFontVariationSettings.bind(this);
this.init();
}
init() {
@@ -103,17 +103,16 @@ class FontInspector {
}, fontsApp);
// Expose the provider to let inspector.js use it in setupSidebar.
this.provider = provider;
this.inspector.selection.on("new-node-front", this.onNewNode);
// @see ToolSidebar.onSidebarTabSelected()
this.inspector.sidebar.on("fontinspector-selected", this.onNewNode);
- this.ruleView.on("property-value-updated", this.onRuleUpdated);
// Listen for theme changes as the color of the previews depend on the theme
gDevTools.on("theme-switched", this.onThemeChanged);
}
/**
* Given all fonts on the page, and given the fonts used in given node, return all fonts
* not from the page not used in this node.
@@ -148,28 +147,76 @@ 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.
*
* @return {Object}
*/
getFontProperties() {
const KEYWORD_VALUES = ["initial", "inherit", "unset", "none"];
const properties = {};
- // First, get all expected font properties from computed styles.
+ // First, get all expected font properties from computed styles, if available.
for (const prop of FONT_PROPERTIES) {
- properties[prop] = this.nodeComputedStyle[prop].value;
+ properties[prop] =
+ (this.nodeComputedStyle[prop] && this.nodeComputedStyle[prop].value)
+ ? this.nodeComputedStyle[prop].value
+ : "";
}
// Then, replace with enabled font properties found on any of the rules that apply.
for (const rule of this.ruleView.rules) {
for (const textProp of rule.textProps) {
if (FONT_PROPERTIES.includes(textProp.name) &&
!KEYWORD_VALUES.includes(textProp.value) &&
!textProp.value.includes("calc(") &&
@@ -338,31 +385,61 @@ 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
+ .map(family => family.toLowerCase())
+ .filter(family => !familiesUsedLowercase.includes(family));
+
+ return families;
+ }
+
+ /**
* Check if the font inspector panel is visible.
*
* @return {Boolean}
*/
isPanelVisible() {
- return this.inspector.sidebar &&
+ return this.inspector &&
+ this.inspector.sidebar &&
this.inspector.sidebar.getCurrentTabID() === "fontinspector";
}
/**
* Check if a selected node exists and fonts can apply to it.
*
* @return {Boolean}
*/
isSelectedNodeValid() {
- return this.inspector.selection.nodeFront &&
+ return this.inspector &&
+ this.inspector.selection.nodeFront &&
this.inspector.selection.isConnected() &&
this.inspector.selection.isElementNode();
}
/**
* Sync the Rule view with the latest styles from the page. Called in a debounced way
* (see constructor) after property changes are applied directly to the CSS style rule
* on the page circumventing direct TextProperty.setValue() which triggers expensive DOM
@@ -572,45 +649,44 @@ class FontInspector {
this.store.dispatch(resetFontEditor());
return;
}
// Get computed styles for the selected node, but filter by CSS font properties.
this.nodeComputedStyle = await this.pageStyle.getComputed(node, {
filterProperties: FONT_PROPERTIES
});
- // Clear any references to writer methods because the node's
+
+ if (!this.nodeComputedStyle) {
+ this.store.dispatch(resetFontEditor());
+ return;
+ }
+
+ // Clear any references to writer methods and CSS declarations because the node's
// styles may have changed since the last font editor refresh.
this.writers.clear();
// 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 fontEditor = this.store.getState().fontEditor;
+
const properties = this.getFontProperties();
- // Names of fonts declared in font-family property without quotes and space trimmed.
- const declaredFontNames =
- properties["font-family"].split(",").map(font => font.replace(/\"+/g, "").trim());
-
- // Mark available fonts as used if their names appears in the font-family declaration.
- // TODO: sort used fonts in order of font-family declaration.
- for (const font of fonts) {
- font.used = declaredFontNames.includes(font.CSSFamilyName);
- }
-
+ 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.
+ const 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));
});
- // Update the font editor state only if property values differ from the ones in store.
- // This can happen when a user makes manual changes in the Rule view.
- if (JSON.stringify(properties) !== JSON.stringify(fontEditor.properties)) {
- this.store.dispatch(updateFontEditor(fonts, properties));
- }
+ this.store.dispatch(updateFontEditor(fontsUsed, families, properties));
}
/**
* 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.
* This method is debounced. See constructor.
*/
snapshotChanges() {
--- a/devtools/client/inspector/fonts/reducers/font-editor.js
+++ b/devtools/client/inspector/fonts/reducers/font-editor.js
@@ -16,17 +16,25 @@ const {
UPDATE_PROPERTY_VALUE,
} = 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: [],
- // Fonts applicable to selected element.
+ // 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: [],
// Current selected font variation instance.
instance: {
name: getStr("fontinspector.customInstanceName"),
values: [],
},
// CSS font properties defined on the selected rule.
properties: {},
@@ -65,17 +73,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, properties }) {
+ [UPDATE_EDITOR_STATE](state, { fonts, families, properties }) {
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;
}
@@ -85,17 +93,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, properties };
+ return { ...state, axes, fonts, families, properties };
},
[UPDATE_PROPERTY_VALUE](state, { property, value }) {
const newState = { ...state };
newState.properties[property] = value;
return newState;
}
--- a/devtools/client/inspector/fonts/types.js
+++ b/devtools/client/inspector/fonts/types.js
@@ -74,25 +74,36 @@ 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,
- // Fonts applicable to selected element
+ // 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: 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/inspector/fonts/utils/font-utils.js
+++ b/devtools/client/inspector/fonts/utils/font-utils.js
@@ -51,27 +51,32 @@ module.exports = {
* Its contents are expected to be stable having been already parsed by the
* browser.
* @return {Object}
*/
parseFontVariationAxes(string) {
let axes = {};
const keywords = ["initial", "normal", "inherit", "unset"];
- if (keywords.includes(string.trim())) {
+ if (!string || keywords.includes(string.trim())) {
return axes;
}
// Parse font-variation-settings CSS declaration into an object
// with axis tags as keys and axis values as values.
axes = string
.split(",")
.reduce((acc, pair) => {
// Tags are always in quotes. Split by quote and filter excessive whitespace.
pair = pair.split(/["']/).filter(part => part.trim() !== "");
+ // Guard against malformed input that may have slipped through.
+ if (pair.length === 0) {
+ return acc;
+ }
+
const tag = pair[0];
const value = pair[1].trim();
// Axis tags shorter or longer than 4 characters are invalid. Whitespace is valid.
if (tag.length === 4) {
acc[tag] = value;
}
return acc;
}, {});
--- a/devtools/server/actors/styles.js
+++ b/devtools/server/actors/styles.js
@@ -295,16 +295,17 @@ var PageStyleActor = protocol.ActorClass
const fonts = InspectorUtils.getUsedFontFaces(rng);
const fontsArray = [];
for (let i = 0; i < fonts.length; i++) {
const font = fonts[i];
const fontFace = {
name: font.name,
CSSFamilyName: font.CSSFamilyName,
+ CSSGeneric: font.CSSGeneric || null,
srcIndex: font.srcIndex,
URI: font.URI,
format: font.format,
localName: font.localName,
metadata: font.metadata
};
// If this font comes from a @font-face rule