Bug 1459898 - (Part 2) Add test for font-size unit conversion. r=gl
MozReview-Commit-ID: JZl7igm6h6A
--- a/devtools/client/inspector/fonts/fonts.js
+++ b/devtools/client/inspector/fonts/fonts.js
@@ -421,24 +421,29 @@ class FontInspector {
}
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.
*
* @param {String} name
* CSS property name
* @param {String} value
* CSS property value
- * @return {TextProperty}
+ * @return {TextProperty|null}
*/
getTextProperty(name, value) {
+ if (!this.selectedRule) {
+ return null;
+ }
+
let textProperty =
this.selectedRule.textProps.find(prop => prop.name === name);
if (!textProperty) {
textProperty = this.selectedRule.editor.addProperty(name, value, "", true);
}
return textProperty;
}
@@ -610,17 +615,24 @@ class FontInspector {
* @param {String} name
* CSS property name
* @param {String} value
* CSS property value
*/
syncChanges(name, value) {
const textProperty = this.getTextProperty(name, value);
if (textProperty) {
- textProperty.setValue(value);
+ // This method may be called after the connection to the page style actor is closed.
+ // For example, during teardown of automated tests. Here, we catch any failure that
+ // may occur because of that. We're not interested in handling the error.
+ try {
+ textProperty.setValue(value);
+ } catch (e) {
+ // Silent error.
+ }
}
this.ruleView.on("property-value-updated", this.onRulePropertyUpdated);
}
/**
* Handler for changes of a font axis value coming from the FontEditor.
*
@@ -709,18 +721,22 @@ class FontInspector {
* @param {String|undefined} toUnit
* Optional CSS unit to convert to
*/
async onPropertyChange(property, value, fromUnit, toUnit) {
if (FONT_PROPERTIES.includes(property)) {
let unit = fromUnit;
if (toUnit && fromUnit) {
- value = await this.convert(value, fromUnit, toUnit);
- unit = toUnit;
+ try {
+ value = await this.convertUnits(value, fromUnit, toUnit);
+ unit = toUnit;
+ } catch (err) {
+ // Silent error
+ }
}
this.onFontPropertyUpdate(property, value, unit);
} else {
this.onAxisUpdate(property, value);
}
}
@@ -984,15 +1000,21 @@ class FontInspector {
const textProperty = this.getTextProperty(name, value);
if (!textProperty || 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.
- textProperty.rule.previewPropertyValue(textProperty, value, "");
+
+ try {
+ textProperty.rule.previewPropertyValue(textProperty, value, "");
+ } catch (e) {
+ // Silent error
+ }
+
// Sync Rule view with changes reflected on the page (debounced).
this.syncChanges(name, value);
}
}
module.exports = FontInspector;
--- a/devtools/client/inspector/fonts/test/browser.ini
+++ b/devtools/client/inspector/fonts/test/browser.ini
@@ -15,16 +15,17 @@ support-files =
!/devtools/client/shared/test/test-actor.js
!/devtools/client/shared/test/test-actor-registry.js
[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_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_rendered-fonts.js]
[browser_fontinspector_reveal-in-page.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/fonts/test/browser_fontinspector_editor-font-size-conversion.js
@@ -0,0 +1,71 @@
+/* vim: set ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+/* global getPropertyValue waitFor */
+
+"use strict";
+
+// Test that changes to font-size units converts the value to the destination unit.
+// Check that converted values are applied to the inline style of the selected element.
+
+const TEST_URI = URL_ROOT + "browser_fontinspector.html";
+
+add_task(async function() {
+ await pushPref("devtools.inspector.fonteditor.enabled", true);
+ const { inspector, view, tab, testActor } = await openFontInspectorForURL(TEST_URI);
+ const viewDoc = view.document;
+ const property = "font-size";
+ const selector = "div";
+ const UNITS = {
+ "px": "36px",
+ "%": "100%",
+ "em": "1em",
+ };
+
+ await selectNode(selector, inspector);
+
+ info("Check that font editor shows font-size value in original units");
+ let fontSize = getPropertyValue(viewDoc, property);
+ is(fontSize.unit, "em", "Original unit for font size is em");
+ is(fontSize.value + fontSize.unit, "1em", "Original font size is 1em");
+
+ let prevValue = fontSize.value;
+ let prevUnit = fontSize.unit;
+
+ for (const unit in UNITS) {
+ const value = UNITS[unit];
+ const onEditorUpdated = inspector.once("fonteditor-updated");
+
+ info(`Convert font-size from ${prevValue}${prevUnit} to ${unit}`);
+ await view.onPropertyChange(property, prevValue, prevUnit, unit);
+ info("Waiting for font editor to re-render");
+ await onEditorUpdated;
+ info(`Waiting for font-size unit dropdown to re-render with selected value: ${unit}`);
+ await waitFor(() => {
+ const sel = `#font-editor .font-value-slider[name=${property}] ~ .font-unit-select`;
+ return viewDoc.querySelector(sel).value === unit;
+ });
+ info("Waiting for testactor reflow");
+ await testActor.reflow();
+
+ info(`Check that font editor font-size value is converted to ${unit}`);
+ fontSize = getPropertyValue(viewDoc, property);
+ is(fontSize.unit, unit, `Font size unit is converted to ${unit}`);
+ is(fontSize.value + fontSize.unit, value, `Font size in font editor is ${value}`);
+
+ info(`Check that inline style font-size value is converted to ${unit}`);
+ const inlineStyleValue = await getInlineStyleValue(tab, selector, property);
+ is(inlineStyleValue, value, `Font size on inline style is ${value}`);
+
+ // Store current unit and value to use in conversion on the next iteration.
+ prevUnit = fontSize.unit;
+ prevValue = fontSize.value;
+ }
+});
+
+async function getInlineStyleValue(tab, selector, property) {
+ return ContentTask.spawn(tab.linkedBrowser, { selector, property }, function(args) {
+ const el = content.document.querySelector(args.selector);
+ return el && el.style[args.property];
+ });
+}
--- a/devtools/client/inspector/fonts/test/browser_fontinspector_editor-keywords.js
+++ b/devtools/client/inspector/fonts/test/browser_fontinspector_editor-keywords.js
@@ -1,37 +1,28 @@
/* vim: set ts=2 et sw=2 tw=80: */
/* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */
+/* global getPropertyValue */
"use strict";
// Test that keyword values for font properties don't show up in the font editor,
// but their computed style values show up instead.
const TEST_URI = URL_ROOT + "browser_fontinspector.html";
add_task(async function() {
await pushPref("devtools.inspector.fonteditor.enabled", true);
const { inspector, view } = await openFontInspectorForURL(TEST_URI);
const viewDoc = view.document;
await testKeywordValues(inspector, viewDoc);
});
-function getPropertyValue(viewDoc, name) {
- const selector = `#font-editor .font-value-slider[name=${name}]`;
- return {
- value: viewDoc.querySelector(selector).value,
- // Ensure unit dropdown exists before querying its value
- unit: viewDoc.querySelector(selector + ` ~ .font-unit-select`) &&
- viewDoc.querySelector(selector + ` ~ .font-unit-select`).value
- };
-}
-
async function testKeywordValues(inspector, viewDoc) {
await selectNode(".bold-text", inspector);
info("Check font-weight shows its computed style instead of the bold keyword value.");
const fontWeight = getPropertyValue(viewDoc, "font-weight");
isnot(fontWeight.value, "bold", "Font weight is not shown as keyword");
is(fontWeight.value, "700", "Font weight is shown as computed style");
--- a/devtools/client/inspector/fonts/test/head.js
+++ b/devtools/client/inspector/fonts/test/head.js
@@ -32,25 +32,26 @@ selectNode = async function(node, inspec
/**
* Adds a new tab with the given URL, opens the inspector and selects the
* font-inspector tab.
* @return {Promise} resolves to a {toolbox, inspector, view} object
*/
var openFontInspectorForURL = async function(url) {
const tab = await addTab(url);
- const {toolbox, inspector} = await openInspector();
+ const {toolbox, inspector, testActor } = await openInspector();
// Call selectNode again here to force a fontinspector update since we don't
// know if the fontinspector-updated event has been sent while the inspector
// was being opened or not.
await selectNode("body", inspector);
return {
tab,
+ testActor,
toolbox,
inspector,
view: inspector.fontinspector
};
};
/**
* Focus one of the preview inputs, clear it, type new text into it and wait for the
@@ -199,8 +200,50 @@ function getURL(fontEl) {
* @param {DOMNode} fontEl
* The font element.
* @return {String}
* The name of the font family as shown in the UI.
*/
function getFamilyName(fontEl) {
return fontEl.querySelector(".font-family-name").textContent;
}
+
+/**
+ * Get the value and unit of a CSS font property or font axis from the font editor.
+ *
+ * @param {document} viewDoc
+ * Host document of the font inspector panel.
+ * @param {String} name
+ * Font property name or axis tag
+ * @return {Object}
+ * Object with the value and unit of the given font property or axis tag
+ * from the corresponding input fron the font editor.
+ * @Example:
+ * {
+ * value: {Number|String|null}
+ * unit: {String|null}
+ * }
+ */
+function getPropertyValue(viewDoc, name) {
+ const selector = `#font-editor .font-value-slider[name=${name}]`;
+ return {
+ // Ensure value input exists before querying its value
+ value: viewDoc.querySelector(selector) &&
+ parseFloat(viewDoc.querySelector(selector).value),
+ // Ensure unit dropdown exists before querying its value
+ unit: viewDoc.querySelector(selector + ` ~ .font-unit-select`) &&
+ viewDoc.querySelector(selector + ` ~ .font-unit-select`).value
+ };
+}
+
+/**
+ * Wait for a predicate to return a result.
+ *
+ * @param {Function} condition
+ * Invoked every 10ms for a maximum of 500 retries until it returns a truthy
+ * value.
+ * @return {Promise}
+ * A promise that is resolved with the result of the condition.
+ */
+async function waitFor(condition) {
+ await BrowserTestUtils.waitForCondition(condition, "waitFor", 10, 500);
+ return condition();
+}