Bug 1159922 - In rule view, update the computed values of a property after editing. r=pbro

browser/devtools/styleinspector/rule-view.js

@@ -592,13 +592,8 @@ Rule.prototype = {
    * Reapply all the properties in this rule, and update their
    * computed styles. Store disabled properties in the element
    * style's store. Will re-mark overridden properties.
-   *
-   * @param {string} [aName]
-   *        A text property name (such as "background" or "border-top") used
-   *        when calling from setPropertyValue & setPropertyName to signify
-   *        that the property should be saved in store.userProperties.
    */
-  applyProperties: function(aModifications, aName) {
+  applyProperties: function(aModifications) {
     this.elementStyle.markOverriddenAll();
 
     if (!aModifications) {
@@ -718,8 +713,6 @@ Rule.prototype = {
    *        The property's priority (either "important" or an empty string).
    */
   previewPropertyValue: function(aProperty, aValue, aPriority) {
-    aProperty.value = aValue;
-
     let modifications = this.style.startModifyingProperties();
     modifications.setProperty(aProperty.name, aValue, aPriority);
     modifications.apply();
@@ -3359,14 +3352,12 @@ TextPropertyEditor.prototype = {
       this.valueSpan.querySelectorAll("." + colorSwatchClass);
     if (this.ruleEditor.isEditable) {
       for (let span of this._colorSwatchSpans) {
-        // Capture the original declaration value to be able to revert later
-        let originalValue = this.valueSpan.textContent;
         // Adding this swatch to the list of swatches our colorpicker
         // knows about
         this.ruleEditor.ruleView.tooltips.colorPicker.addSwatch(span, {
           onPreview: () => this._previewValue(this.valueSpan.textContent),
-          onCommit: () => this._applyNewValue(this.valueSpan.textContent),
-          onRevert: () => this._applyNewValue(originalValue, false)
+          onCommit: () => this._onValueDone(this.valueSpan.textContent, true),
+          onRevert: () => this._onValueDone(undefined, false)
         });
       }
     }
@@ -3376,14 +3367,12 @@ TextPropertyEditor.prototype = {
       this.valueSpan.querySelectorAll("." + bezierSwatchClass);
     if (this.ruleEditor.isEditable) {
       for (let span of this._bezierSwatchSpans) {
-        // Capture the original declaration value to be able to revert later
-        let originalValue = this.valueSpan.textContent;
         // Adding this swatch to the list of swatches our colorpicker
         // knows about
         this.ruleEditor.ruleView.tooltips.cubicBezier.addSwatch(span, {
           onPreview: () => this._previewValue(this.valueSpan.textContent),
-          onCommit: () => this._applyNewValue(this.valueSpan.textContent),
-          onRevert: () => this._applyNewValue(originalValue, false)
+          onCommit: () => this._onValueDone(this.valueSpan.textContent, true),
+          onRevert: () => this._onValueDone(undefined, false)
         });
       }
     }
@@ -3393,12 +3382,11 @@ TextPropertyEditor.prototype = {
     if (this.ruleEditor.isEditable) {
       if (span) {
         parserOptions.filterSwatch = true;
-        let originalValue = this.valueSpan.textContent;
 
         this.ruleEditor.ruleView.tooltips.filterEditor.addSwatch(span, {
           onPreview: () => this._previewValue(this.valueSpan.textContent),
-          onCommit: () => this._applyNewValue(this.valueSpan.textContent),
-          onRevert: () => this._applyNewValue(originalValue, false)
+          onCommit: () => this._onValueDone(this.valueSpan.textContent, true),
+          onRevert: () => this._onValueDone(undefined, false)
         }, outputParser, parserOptions);
       }
     }
@@ -3600,7 +3588,12 @@ TextPropertyEditor.prototype = {
       if (this.removeOnRevert) {
         this.remove();
       } else {
-        this.prop.setValue(this.committed.value, this.committed.priority);
+        // update the editor back to committed value
+        this.update();
+
+        // undo the preview in content style
+        this.ruleEditor.rule.previewPropertyValue(this.prop,
+          this.prop.value, this.prop.priority);
       }
       return;
     }
@@ -3680,32 +3673,6 @@ TextPropertyEditor.prototype = {
     };
   },
 
-  /**
-   * Apply a new value.
-   *
-   * @param  {String} aValue
-   *         The value to replace.
-   * @param  {Boolean} markChanged=true
-   *         Set this to false if you need to prevent the property from being
-   *         marked as changed e.g. tooltips do this when <escape> is pressed
-   *         in order to revert the value.
-   */
-  _applyNewValue: function(aValue, markChanged=true) {
-    let val = parseSingleValue(aValue);
-
-    if (!markChanged) {
-      let store = this.prop.rule.elementStyle.store;
-      this.prop.editor.committed.value = aValue;
-      store.userProperties.setProperty(this.prop.rule.style,
-                                       this.prop.rule.name, aValue);
-    }
-
-    this.prop.setValue(val.value, val.priority, markChanged);
-    this.removeOnRevert = false;
-    this.committed.value = this.prop.value;
-    this.committed.priority = this.prop.priority;
-  },
-
   /**
    * Live preview this property, without committing changes.
    * @param {string} aValue The value to set the current property to.

browser/devtools/styleinspector/test/browser.ini

@@ -84,6 +84,7 @@ skip-if = e10s # Bug 1039528: "inspect element" contextual-menu doesn't work wit
 [browser_ruleview_cubicbezier-commit-on-ENTER.js]
 [browser_ruleview_cubicbezier-revert-on-ESC.js]
 [browser_ruleview_edit-property-commit.js]
+[browser_ruleview_edit-property-computed.js]
 [browser_ruleview_edit-property-increments.js]
 [browser_ruleview_edit-property-order.js]
 [browser_ruleview_edit-property_01.js]

browser/devtools/styleinspector/test/browser_ruleview_edit-property-computed.js

@@ -0,0 +1,67 @@
+/* vim: set ft=javascript 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 computed values of a style (the shorthand expansion) are
+// properly updated after the style is changed.
+add_task(function*() {
+  let TEST_URI = [
+    '<style type="text/css">',
+    '  #testid {',
+    '    padding: 10px;',
+    '  }',
+    '</style>',
+    '<div id="testid">Styled Node</div>'
+  ].join('\n');
+
+  yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
+  let {inspector, view} = yield openRuleView();
+  yield selectNode("#testid", inspector);
+  yield editAndCheck(view);
+});
+
+function* editAndCheck(view) {
+  let idRuleEditor = getRuleViewRuleEditor(view, 1);
+  let prop = idRuleEditor.rule.textProps[0];
+  let propEditor = prop.editor;
+  let newPaddingValue = "20px";
+
+  info("Focusing the inplace editor field");
+  let editor = yield focusEditableField(propEditor.valueSpan);
+  is(inplaceEditor(propEditor.valueSpan), editor, "Focused editor should be the value span.");
+
+  let onPropertyChange = waitForComputedStyleProperty("#testid", null, "padding-top", newPaddingValue);
+
+  info("Entering a new value");
+  EventUtils.sendString(newPaddingValue, view.doc.defaultView);
+
+  info("Waiting for the throttled previewValue to apply the changes to document");
+  yield onPropertyChange;
+
+  let onBlur = once(editor.input, "blur");
+
+  info("Entering the commit key and finishing edit");
+  EventUtils.synthesizeKey("VK_RETURN", {});
+
+  info("Waiting for blur on the field");
+  yield onBlur;
+
+  info("Waiting for the style changes to be applied");
+  yield once(view, "ruleview-changed");
+
+  let computed = prop.computed;
+  let propNames = [
+    "padding-top",
+    "padding-right",
+    "padding-bottom",
+    "padding-left"
+  ];
+
+  is(computed.length, propNames.length, "There should be 4 computed values");
+  propNames.forEach((propName, i) => {
+    is(computed[i].name, propName, "Computed property #" + i + " has name " + propName);
+    is(computed[i].value, newPaddingValue, "Computed value of " + propName + " is as expected");
+  });
+}