Bug 1475782 - Fix for intermittent test failures on Font Editor unit conversion. r=pbro
- Catch getComputed errors from async call
- Explicitly create new TextProperty when none exists
MozReview-Commit-ID: DokLJHIO6Cr
--- a/devtools/client/inspector/fonts/fonts.js
+++ b/devtools/client/inspector/fonts/fonts.js
@@ -209,32 +209,49 @@ class FontInspector {
if (unit === "pc") {
out = fromPx
? value * 0.0625
: value / 0.0625;
}
if (unit === "%") {
- computedStyle = await this.pageStyle.getComputed(node.parentNode());
+ computedStyle =
+ await this.pageStyle.getComputed(node.parentNode()).catch(console.error);
+
+ if (!computedStyle) {
+ return value;
+ }
+
out = fromPx
? value * 100 / parseFloat(computedStyle["font-size"].value)
: value / 100 * parseFloat(computedStyle["font-size"].value);
}
if (unit === "em") {
- computedStyle = await this.pageStyle.getComputed(node.parentNode());
+ computedStyle =
+ await this.pageStyle.getComputed(node.parentNode()).catch(console.error);
+
+ if (!computedStyle) {
+ return value;
+ }
+
out = fromPx
? value / parseFloat(computedStyle["font-size"].value)
: value * parseFloat(computedStyle["font-size"].value);
}
if (unit === "rem") {
const document = await this.inspector.walker.documentElement();
- computedStyle = await this.pageStyle.getComputed(document);
+ computedStyle = await this.pageStyle.getComputed(document).catch(console.error);
+
+ if (!computedStyle) {
+ return value;
+ }
+
out = fromPx
? value / parseFloat(computedStyle["font-size"].value)
: value * parseFloat(computedStyle["font-size"].value);
}
if (unit === "vh" || unit === "vw" || unit === "vmin" || unit === "vmax") {
const dim = await node.getOwnerGlobalDimensions();
@@ -418,39 +435,30 @@ class FontInspector {
allFonts = [];
}
return this.excludeNodeFonts(allFonts, nodeFonts);
}
/**
* Get a reference to a TextProperty instance from the current selected rule for a
- * given property name. If one doesn't exist, create one with the given value.
- * If the selected rule no longer exists (ex: during test teardown), return null.
+ * given property name.
*
* @param {String} name
* CSS property name
- * @param {String} value
- * CSS property value
* @return {TextProperty|null}
*/
- getTextProperty(name, value) {
+ getTextProperty(name) {
if (!this.selectedRule) {
return null;
}
- let textProperty =
- this.selectedRule.textProps.find(prop =>
- prop.name === name && prop.enabled && !prop.overridden
- );
- if (!textProperty) {
- textProperty = this.selectedRule.editor.addProperty(name, value, "", true);
- }
-
- return textProperty;
+ return this.selectedRule.textProps.find(prop =>
+ prop.name === name && prop.enabled && !prop.overridden
+ );
}
/**
* Given the axis name of a registered axis, return a method which updates the
* corresponding CSS font property when called with a value.
*
* All variable font axes can be written in the value of the "font-variation-settings"
* CSS font property. In CSS Fonts Level 4, registered axes values can be used as
@@ -766,21 +774,23 @@ class FontInspector {
}
}
/**
* Handler for "property-value-updated" event emitted from the rule view whenever a
* property value changes. Ignore changes to properties unrelated to the font editor.
*
* @param {Object} eventData
- * Object with the property name and value.
- * Example: { name: "font-size", value: "1em" }
+ * Object with the property name and value and origin rule.
+ * Example: { name: "font-size", value: "1em", rule: Object }
*/
async onRulePropertyUpdated(eventData) {
- if (!FONT_PROPERTIES.includes(eventData.property)) {
+ if (!this.selectedRule ||
+ !this.selectedRule.matches({ rule: eventData.rule.domRule }) ||
+ !FONT_PROPERTIES.includes(eventData.property)) {
return;
}
if (this.isPanelVisible()) {
await this.refreshFontEditor();
}
}
@@ -877,20 +887,31 @@ 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 computed styles for the selected node, but filter by CSS font properties.
- this.nodeComputedStyle = await this.pageStyle.getComputed(node, {
- filterProperties: FONT_PROPERTIES
- });
+ try {
+ // Get computed styles for the selected node, but filter by CSS font properties.
+ this.nodeComputedStyle = await this.pageStyle.getComputed(node, {
+ filterProperties: FONT_PROPERTIES
+ });
+ } catch (e) {
+ // Because getComputed is async, there is a chance the font editor was
+ // destroyed while we were waiting. If that happened, just bail out
+ // silently.
+ if (!this.document) {
+ return;
+ }
+
+ throw e;
+ }
if (!this.nodeComputedStyle || !fonts.length) {
this.store.dispatch(resetFontEditor());
this.inspector.emit("fonteditor-updated");
return;
}
// Clear any references to writer methods and CSS declarations because the node's
@@ -1032,29 +1053,30 @@ class FontInspector {
* as "changed" in the Rule view with a green indicator.
*
* @param {String} name
* CSS property name
* @param {String}value
* CSS property value
*/
updatePropertyValue(name, value) {
- const textProperty = this.getTextProperty(name, value);
- if (!textProperty || textProperty.value === value) {
+ const textProperty = this.getTextProperty(name);
+
+ if (!textProperty) {
+ this.selectedRule.createProperty(name, value, "", true);
+ return;
+ }
+
+ if (textProperty.value === value) {
return;
}
// Prevent reacting to changes we caused.
this.ruleView.off("property-value-updated", this.onRulePropertyUpdated);
// Live preview font property changes on the page.
-
- try {
- textProperty.rule.previewPropertyValue(textProperty, value, "");
- } catch (e) {
- // Silent error
- }
+ textProperty.rule.previewPropertyValue(textProperty, value, "");
// Sync Rule view with changes reflected on the page (debounced).
this.syncChanges(name, value);
}
}
module.exports = FontInspector;
--- a/devtools/client/inspector/rules/views/text-property-editor.js
+++ b/devtools/client/inspector/rules/views/text-property-editor.js
@@ -418,17 +418,23 @@ TextPropertyEditor.prototype = {
value: frag.textContent,
priority: this.prop.priority,
};
}
this.valueSpan.innerHTML = "";
this.valueSpan.appendChild(frag);
- this.ruleView.emit("property-value-updated", { property: name, value: val });
+ this.ruleView.emit("property-value-updated",
+ {
+ rule: this.prop.rule,
+ property: name,
+ value: val,
+ }
+ );
// Highlight the currently used font in font-family properties.
// If we cannot find a match, highlight the first generic family instead.
const fontFamilySpans = this.valueSpan.querySelectorAll("." + FONT_FAMILY_CLASS);
if (fontFamilySpans.length && this.prop.enabled && !this.prop.overridden) {
this.rule.elementStyle.getUsedFontFamilies().then(families => {
const usedFontFamilies = families.map(font => font.toLowerCase());
let foundMatchingFamily = false;