Bug 1463055 - Font editor: read properties from computed style; write to inline style. r=pbro
MozReview-Commit-ID: 6sHeBgfbiDT
--- a/devtools/client/inspector/fonts/fonts.js
+++ b/devtools/client/inspector/fonts/fonts.js
@@ -8,16 +8,17 @@
const Services = require("Services");
const { gDevTools } = require("devtools/client/framework/devtools");
const { getColor } = require("devtools/client/shared/theme");
const { createFactory, createElement } = require("devtools/client/shared/vendor/react");
const { Provider } = require("devtools/client/shared/vendor/react-redux");
const { throttle } = require("devtools/shared/throttle");
const { debounce } = require("devtools/shared/debounce");
+const { ELEMENT_STYLE } = require("devtools/shared/specs/styles");
const FontsApp = createFactory(require("./components/FontsApp"));
const { LocalizationHelper } = require("devtools/shared/l10n");
const INSPECTOR_L10N =
new LocalizationHelper("devtools/client/locales/inspector.properties");
const { getStr } = require("./utils/l10n");
@@ -152,39 +153,27 @@ class FontInspector {
this.store = null;
this.textProperties.clear();
this.textProperties = null;
this.writers.clear();
this.writers = null;
}
/**
- * Collect all the expected CSS font properties and their values for the selected node
- * from the node's computed style and from all the rules that apply to it.
+ * Get all expected CSS font properties and values from the node's computed style.
*
* @return {Object}
*/
getFontProperties() {
let properties = {};
- // First, get all expected font properties from computed styles.
for (let prop of FONT_PROPERTIES) {
properties[prop] = this.nodeComputedStyle[prop].value;
}
- // Then, replace with enabled font properties found on any of the rules that apply.
- for (let rule of this.ruleView.rules) {
- for (let textProp of rule.textProps) {
- if (FONT_PROPERTIES.includes(textProp.name) && textProp.enabled &&
- !textProp.overridden) {
- properties[textProp.name] = textProp.value;
- }
- }
- }
-
return properties;
}
async getFontsForNode(node, options) {
// In case we've been destroyed in the meantime
if (!this.document) {
return [];
}
@@ -338,17 +327,19 @@ class FontInspector {
} else {
this.writers.set(name, this.updateFontVariationSettings);
}
return this.writers.get(name);
}
/**
- * Returns true if the font inspector panel is visible, and false otherwise.
+ * Check if the font inspector panel is visible.
+ *
+ * @return {Boolean}
*/
isPanelVisible() {
return this.inspector.sidebar &&
this.inspector.sidebar.getCurrentTabID() === "fontinspector";
}
/**
* Check if a selected node exists and fonts can apply to it.
*
@@ -527,22 +518,24 @@ class FontInspector {
if (frame === this.document.defaultView) {
this.update();
}
}
/**
* Update the state of the font editor with:
* - the fonts which apply to the current node;
- * - the CSS font properties declared on the selected rule.
+ * - the computed style CSS font properties of the current node.
*
- * This method is called during initial setup and as a result of any property
- * values change in the Rule view. For the latter case, we do a deep compare between the
- * font properties on the selected rule and the ones already store to decide if to
- * update the font edtior to reflect a new external state.
+ * This method is called:
+ * - during initial setup;
+ * - when a new node is selected;
+ * - when any property is changed in the Rule view.
+ * For the latter case, we compare between the latest computed style font properties
+ * and the ones already in the store to decide if to update the font editor state.
*/
async refreshFontEditor() {
// Early return if pref for font editor is not enabled.
if (!Services.prefs.getBoolPref(PREF_FONT_EDITOR)) {
return;
}
if (!this.inspector || !this.store || !this.isSelectedNodeValid()) {
@@ -552,25 +545,32 @@ class FontInspector {
const options = {};
if (this.pageStyle.supportsFontVariations) {
options.includeVariations = true;
}
const node = this.inspector.selection.nodeFront;
const fonts = await this.getFontsForNode(node, options);
- // Get all computed styles for selected node; used for identifying inherited values.
+ // 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 and CSS declarations because the node's
+ // styles may have changed since the last font editor refresh.
+ this.writers.clear();
+ this.textProperties.clear();
+ // Select the node's inline style as the rule where to write property value changes.
+ this.selectedRule =
+ this.ruleView.rules.find(rule => rule.style.type === ELEMENT_STYLE);
const fontEditor = this.store.getState().fontEditor;
const properties = this.getFontProperties();
- // Update the font editor state only if property values in rule differ from store.
- // This can happen when a user makes manual edits to the values in the rule view.
+ // 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));
}
}
/**
* 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.
@@ -589,24 +589,17 @@ class FontInspector {
let node = this.inspector.selection.nodeFront;
let fonts = [];
let otherFonts = [];
let { fontOptions } = this.store.getState();
let { previewText } = fontOptions;
- // Clear the list of fonts if the currently selected node is not connected or a text
- // or element node unless all fonts are supposed to be shown.
- let isElementOrTextNode = this.inspector.selection.isElementNode() ||
- this.inspector.selection.isTextNode();
- if (!node ||
- !this.isPanelVisible() ||
- !this.inspector.selection.isConnected() ||
- !isElementOrTextNode) {
+ if (!this.isSelectedNodeValid() || !this.isPanelVisible()) {
this.store.dispatch(updateFonts(fonts, otherFonts));
return;
}
let options = {
includePreviews: true,
previewText,
previewFillStyle: getColor("body-color")
--- a/devtools/client/inspector/fonts/test/browser.ini
+++ b/devtools/client/inspector/fonts/test/browser.ini
@@ -18,10 +18,9 @@ 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_expand-css-code.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_text-node.js
+++ /dev/null
@@ -1,22 +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";
-
-// Test that the font-inspector also display fonts when a text node is selected.
-
-const TEST_URI = URL_ROOT + "browser_fontinspector.html";
-
-add_task(async function() {
- let { inspector, view } = await openFontInspectorForURL(TEST_URI);
- let viewDoc = view.document;
-
- info("Selecting the first text-node first-child of <body>");
- let bodyNode = await getNodeFront("body", inspector);
- let { nodes } = await inspector.walker.children(bodyNode);
- await selectNode(nodes[0], inspector);
-
- let lis = getUsedFontsEls(viewDoc);
- is(lis.length, 1, "Font inspector shows 1 font");
-});