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

--HG--
extra : rebase_source : fca1d8e9a3161eb8324d47b95e7862859fd42e1d

devtools/client/inspector/fonts/fonts.js

@@ -214,14 +214,26 @@ class FontInspector {
     }
 
     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);
@@ -229,7 +241,12 @@ class FontInspector {
 
     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);
@@ -423,29 +440,20 @@ class FontInspector {
 
   /**
    * 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.
+   * given property name.
-   * 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|null}
    */
-  getTextProperty(name, value) {
+  getTextProperty(name) {
     if (!this.selectedRule) {
       return null;
     }
 
-    let textProperty =
+    return this.selectedRule.textProps.find(prop =>
-      this.selectedRule.textProps.find(prop =>
+      prop.name === name && prop.enabled && !prop.overridden
-        prop.name === name && prop.enabled && !prop.overridden
+    );
-      );
-    if (!textProperty) {
-      textProperty = this.selectedRule.editor.addProperty(name, value, "", true);
-    }
-
-    return textProperty;
   }
 
   /**
@@ -771,11 +779,13 @@ class FontInspector {
    * property value changes. Ignore changes to properties unrelated to the font editor.
    *
    * @param {Object} eventData
-   *        Object with the property name and value.
+   *        Object with the property name and value and origin rule.
-   *        Example: { name: "font-size", value: "1em" }
+   *        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;
     }
 
@@ -882,10 +892,21 @@ class FontInspector {
     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.
+    try {
-    this.nodeComputedStyle = await this.pageStyle.getComputed(node, {
+      // Get computed styles for the selected node, but filter by CSS font properties.
-      filterProperties: 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());
@@ -1037,20 +1058,21 @@ class FontInspector {
    *        CSS property value
    */
   updatePropertyValue(name, value) {
-    const textProperty = this.getTextProperty(name, value);
+    const textProperty = this.getTextProperty(name);
-    if (!textProperty || textProperty.value === value) {
+
+    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.
-
+    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);

devtools/client/inspector/rules/views/text-property-editor.js

@@ -423,7 +423,13 @@ TextPropertyEditor.prototype = {
     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.