From f06270e506c7c70c1d770cdd6e9ae9d233b7363f Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Thu, 11 Jun 2015 12:18:53 +0200 Subject: [PATCH] Bug 1159922 - In rule view, update the computed values of a property after editing. r=pbro --- browser/devtools/styleinspector/rule-view.js | 59 ++++------------ .../devtools/styleinspector/test/browser.ini | 1 + ...browser_ruleview_edit-property-computed.js | 67 +++++++++++++++++++ 3 files changed, 81 insertions(+), 46 deletions(-) create mode 100644 browser/devtools/styleinspector/test/browser_ruleview_edit-property-computed.js diff --git a/browser/devtools/styleinspector/rule-view.js b/browser/devtools/styleinspector/rule-view.js index 234e0cb2d463..7be5914e6130 100644 --- a/browser/devtools/styleinspector/rule-view.js +++ b/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 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. diff --git a/browser/devtools/styleinspector/test/browser.ini b/browser/devtools/styleinspector/test/browser.ini index f505ce7f86f8..b4acdd47e896 100644 --- a/browser/devtools/styleinspector/test/browser.ini +++ b/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] diff --git a/browser/devtools/styleinspector/test/browser_ruleview_edit-property-computed.js b/browser/devtools/styleinspector/test/browser_ruleview_edit-property-computed.js new file mode 100644 index 000000000000..93d7857bd7c5 --- /dev/null +++ b/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 = [ + '', + '
Styled Node
' + ].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"); + }); +}