corysanin/tubestation
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug 913509 - [rule view] Papercuts - Inconsistent behavior when modifying CSS declarations. r=miker

Browse Source
This commit is contained in:
Brian Grinstead
2013-09-12 11:18:29 -05:00
parent 151571716c
commit 77d5dfd269
6 changed files with 231 additions and 72 deletions
Download Patch File Download Diff File Expand all files Collapse all files

32
browser/devtools/shared/inplace-editor.js
View File

@@ -205,10 +205,9 @@ function InplaceEditor(aOptions, aEvent)
aEvt.stopPropagation();
}, false);
this.warning = aOptions.warning;
this.validate = aOptions.validate;
if (this.warning && this.validate) {
if (this.validate) {
this.input.addEventListener("keyup", this._onKeyup, false);
}
@@ -254,15 +253,15 @@ InplaceEditor.prototype = {
this.elt.style.display = this.originalDisplay;
this.elt.focus();
if (this.destroy) {
this.destroy();
}
this.elt.parentNode.removeChild(this.input);
this.input = null;
delete this.elt.inplaceEditor;
delete this.elt;
if (this.destroy) {
this.destroy();
}
},
/**
@@ -352,7 +351,7 @@ InplaceEditor.prototype = {
this.input.value = newValue.value;
this.input.setSelectionRange(newValue.start, newValue.end);
this.warning.hidden = this.validate(this.input.value);
this._doValidation();
return true;
},
@@ -768,8 +767,10 @@ InplaceEditor.prototype = {
input.value = pre + toComplete + post;
input.setSelectionRange(pre.length, pre.length + toComplete.length);
this._updateSize();
// This emit is mainly for the purpose of making the test flow simpler.
this.emit("after-suggest");
this._doValidation();
}
if (aEvent.keyCode === Ci.nsIDOMKeyEvent.DOM_VK_BACK_SPACE ||
@@ -855,8 +856,6 @@ InplaceEditor.prototype = {
* Handle the input field's keyup event.
*/
_onKeyup: function(aEvent) {
// Validate the entered value.
this.warning.hidden = this.validate(this.input.value);
this._applied = false;
},
@@ -866,9 +865,7 @@ InplaceEditor.prototype = {
_onInput: function InplaceEditor_onInput(aEvent)
{
// Validate the entered value.
if (this.warning && this.validate) {
this.warning.hidden = this.validate(this.input.value);
}
this._doValidation();
// Update size if we're autosizing.
if (this._measurement) {
@@ -881,6 +878,16 @@ InplaceEditor.prototype = {
}
},
/**
* Fire validation callback with current input
*/
_doValidation: function()
{
if (this.validate && this.input) {
this.validate(this.input.value);
}
},
/**
* Handles displaying suggestions based on the current input.
*/
@@ -980,6 +987,7 @@ InplaceEditor.prototype = {
}
// This emit is mainly for the purpose of making the test flow simpler.
this.emit("after-suggest");
this._doValidation();
}, 0);
}
};

187
browser/devtools/styleinspector/rule-view.js
View File

@@ -525,6 +525,8 @@ Rule.prototype = {
*/
applyProperties: function Rule_applyProperties(aModifications, aName)
{
this.elementStyle.markOverriddenAll();
if (!aModifications) {
aModifications = this.style.startModifyingProperties();
}
@@ -540,6 +542,9 @@ Rule.prototype = {
});
continue;
}
if (prop.value.trim() === "") {
continue;
}
aModifications.setProperty(prop.name, prop.value, prop.priority);
@@ -629,6 +634,7 @@ Rule.prototype = {
if (aValue === aProperty.value && aPriority === aProperty.priority) {
return;
}
aProperty.value = aValue;
aProperty.priority = aPriority;
this.applyProperties(null, aProperty.name);
@@ -846,6 +852,45 @@ Rule.prototype = {
return false;
},
/**
* Jump between editable properties in the UI. Will begin editing the next
* name, if possible. If this is the last element in the set, then begin
* editing the previous value. If this is the *only* element in the set,
* then settle for focusing the new property editor.
*
* @param {TextProperty} aTextProperty
* The text property that will be left to focus on a sibling.
*
*/
editClosestTextProperty: function Rule__editClosestTextProperty(aTextProperty)
{
let index = this.textProps.indexOf(aTextProperty);
let previous = false;
// If this is the last element, move to the previous instead of next
if (index === this.textProps.length - 1) {
index = index - 1;
previous = true;
}
else {
index = index + 1;
}
let nextProp = this.textProps[index];
// If possible, begin editing the next name or previous value.
// Otherwise, settle for focusing the new property element.
if (nextProp) {
if (previous) {
nextProp.editor.valueSpan.click();
} else {
nextProp.editor.nameSpan.click();
}
} else {
aTextProperty.rule.editor.closeBrace.focus();
}
}
};
/**
@@ -1273,7 +1318,7 @@ CssRuleView.prototype = {
}
if (!rule.editor) {
new RuleEditor(this, rule);
rule.editor = new RuleEditor(this, rule);
}
this.element.appendChild(rule.editor.element);
@@ -1333,7 +1378,6 @@ function RuleEditor(aRuleView, aRule)
this.ruleView = aRuleView;
this.doc = this.ruleView.doc;
this.rule = aRule;
this.rule.editor = this;
this._onNewProperty = this._onNewProperty.bind(this);
this._newPropertyDestroy = this._newPropertyDestroy.bind(this);
@@ -1397,17 +1441,6 @@ RuleEditor.prototype = {
this.element.addEventListener("mousedown", function() {
this.doc.defaultView.focus();
let editorNodes =
this.doc.querySelectorAll(".styleinspector-propertyeditor");
if (editorNodes) {
for (let node of editorNodes) {
if (node.inplaceEditor) {
node.inplaceEditor._clear();
}
}
}
}.bind(this), false);
this.propertyList = createChild(code, "ul", {
@@ -1466,8 +1499,8 @@ RuleEditor.prototype = {
for (let prop of this.rule.textProps) {
if (!prop.editor) {
new TextPropertyEditor(this, prop);
this.propertyList.appendChild(prop.editor.element);
let editor = new TextPropertyEditor(this, prop);
this.propertyList.appendChild(editor.element);
}
}
},
@@ -1581,6 +1614,7 @@ function TextPropertyEditor(aRuleEditor, aProperty)
this.prop = aProperty;
this.prop.editor = this;
this.browserWindow = this.doc.defaultView.top;
this.removeOnRevert = this.prop.value === "";
let sheet = this.prop.rule.sheet;
let href = sheet ? (sheet.href || sheet.nodeHref) : null;
@@ -1593,12 +1627,16 @@ function TextPropertyEditor(aRuleEditor, aProperty)
this._onStartEditing = this._onStartEditing.bind(this);
this._onNameDone = this._onNameDone.bind(this);
this._onValueDone = this._onValueDone.bind(this);
this._onValidate = throttle(this._livePreview, 10, this, this.browserWindow);
this._create();
this.update();
}
TextPropertyEditor.prototype = {
/**
* Boolean indicating if the name or value is being currently edited.
*/
get editing() {
return !!(this.nameSpan.inplaceEditor || this.valueSpan.inplaceEditor);
},
@@ -1627,13 +1665,13 @@ TextPropertyEditor.prototype = {
this.nameContainer = createChild(this.element, "span", {
class: "ruleview-namecontainer"
});
this.nameContainer.addEventListener("click", function(aEvent) {
this.nameContainer.addEventListener("click", (aEvent) => {
// Clicks within the name shouldn't propagate any further.
aEvent.stopPropagation();
if (aEvent.target === propertyContainer) {
this.nameSpan.click();
}
}.bind(this), false);
}, false);
// Property name, editable when focused. Property name
// is committed when the editor is unfocused.
@@ -1646,6 +1684,7 @@ TextPropertyEditor.prototype = {
start: this._onStartEditing,
element: this.nameSpan,
done: this._onNameDone,
destroy: this.update.bind(this),
advanceChars: ':',
contentType: InplaceEditor.CONTENT_TYPES.CSS_PROPERTY,
popup: this.popup
@@ -1659,13 +1698,13 @@ TextPropertyEditor.prototype = {
let propertyContainer = createChild(this.element, "span", {
class: "ruleview-propertycontainer"
});
propertyContainer.addEventListener("click", function(aEvent) {
propertyContainer.addEventListener("click", (aEvent) => {
// Clicks within the value shouldn't propagate any further.
aEvent.stopPropagation();
if (aEvent.target === propertyContainer) {
this.valueSpan.click();
}
}.bind(this), false);
}, false);
// Property value, editable when focused. Changes to the
// property value are applied as they are typed, and reverted
@@ -1684,8 +1723,8 @@ TextPropertyEditor.prototype = {
appendText(propertyContainer, ";");
this.warning = createChild(this.element, "div", {
hidden: "",
class: "ruleview-warning",
hidden: "",
title: CssLogic.l10n("rule.warning.title"),
});
@@ -1699,8 +1738,8 @@ TextPropertyEditor.prototype = {
start: this._onStartEditing,
element: this.valueSpan,
done: this._onValueDone,
validate: this._validate.bind(this),
warning: this.warning,
destroy: this.update.bind(this),
validate: this._onValidate,
advanceChars: ';',
contentType: InplaceEditor.CONTENT_TYPES.CSS_VALUE,
property: this.prop,
@@ -1751,7 +1790,9 @@ TextPropertyEditor.prototype = {
this.enable.removeAttribute("checked");
}
if (this.prop.overridden && !this.editing) {
this.warning.hidden = this.editing || this.isValid();
if ((this.prop.overridden || !this.prop.enabled) && !this.editing) {
this.element.classList.add("ruleview-overridden");
} else {
this.element.classList.remove("ruleview-overridden");
@@ -1766,7 +1807,6 @@ TextPropertyEditor.prototype = {
if (this.prop.priority) {
val += " !" + this.prop.priority;
}
// Treat URLs differently than other properties.
// Allow the user to click a link to the resource and open it.
let resourceURI = this.getResourceURI();
@@ -1797,8 +1837,6 @@ TextPropertyEditor.prototype = {
this.valueSpan.textContent = val;
}
this.warning.hidden = this._validate();
let store = this.prop.rule.elementStyle.store;
let propDirty = store.userProperties.contains(this.prop.rule.style, name);
if (propDirty) {
@@ -1814,6 +1852,7 @@ TextPropertyEditor.prototype = {
_onStartEditing: function TextPropertyEditor_onStartEditing()
{
this.element.classList.remove("ruleview-overridden");
this._livePreview(this.prop.value);
},
/**
@@ -1906,19 +1945,13 @@ TextPropertyEditor.prototype = {
*/
_onNameDone: function TextPropertyEditor_onNameDone(aValue, aCommit)
{
if (!aCommit) {
if (this.prop.overridden) {
this.element.classList.add("ruleview-overridden");
}
return;
}
if (!aValue) {
this.prop.remove();
this.element.parentNode.removeChild(this.element);
return;
}
if (aCommit) {
if (aValue.trim() === "") {
this.remove();
} else {
this.prop.setName(aValue);
}
}
},
/**
@@ -1938,6 +1971,17 @@ TextPropertyEditor.prototype = {
};
},
/**
* Remove property from style and the editors from DOM.
* Begin editing next available property.
*/
remove: function TextPropertyEditor_remove()
{
this.element.parentNode.removeChild(this.element);
this.ruleEditor.rule.editClosestTextProperty(this.prop);
this.prop.remove();
},
/**
* Called when a value editor closes. If the user pressed escape,
* revert to the value this property had before editing.
@@ -1951,27 +1995,56 @@ TextPropertyEditor.prototype = {
{
if (aCommit) {
let val = this._parseValue(aValue);
// Any property should be removed if has an empty value.
if (val.value.trim() === "") {
this.remove();
} else {
this.prop.setValue(val.value, val.priority);
this.removeOnRevert = false;
this.committed.value = this.prop.value;
this.committed.priority = this.prop.priority;
if (this.prop.overridden) {
this.element.classList.add("ruleview-overridden");
}
} else {
// A new property should be removed when escape is pressed.
if (this.removeOnRevert) {
this.remove();
} else {
this.prop.setValue(this.committed.value, this.committed.priority);
}
}
},
/**
* Validate this property.
* Live preview this property, without committing changes.
*
* @param {string} [aValue]
* Override the actual property value used for validation without
* applying property values e.g. validate as you type.
* The value to set the current property to.
*/
_livePreview: function TextPropertyEditor_livePreview(aValue)
{
// Since function call is throttled, we need to make sure we are still editing
if (!this.editing) {
return;
}
let val = this._parseValue(aValue);
// Live previewing the change without committing just yet, that'll be done in _onValueDone
// If it was not a valid value, apply an empty string to reset the live preview
this.ruleEditor.rule.setPropertyValue(this.prop, val.value, val.priority);
},
/**
* Validate this property. Does it make sense for this value to be assigned
* to this property name? This does not apply the property value
*
* @param {string} [aValue]
* The property value used for validation.
* Defaults to the current value for this.prop
*
* @return {bool} true if the property value is valid, false otherwise.
*/
_validate: function TextPropertyEditor_validate(aValue)
isValid: function TextPropertyEditor_isValid(aValue)
{
let name = this.prop.name;
let value = typeof aValue == "undefined" ? this.prop.value : aValue;
@@ -1981,18 +2054,18 @@ TextPropertyEditor.prototype = {
let prefs = Services.prefs;
// We toggle output of errors whilst the user is typing a property value.
let prefVal = Services.prefs.getBoolPref("layout.css.report_errors");
let prefVal = prefs.getBoolPref("layout.css.report_errors");
prefs.setBoolPref("layout.css.report_errors", false);
let validValue = false;
try {
style.setProperty(name, val.value, val.priority);
// Live previewing the change without committing yet just yet, that'll be done in _onValueDone
this.ruleEditor.rule.setPropertyValue(this.prop, val.value, val.priority);
validValue = style.getPropertyValue(name) !== "" || val.value === "";
} finally {
prefs.setBoolPref("layout.css.report_errors", prefVal);
}
return !!style.getPropertyValue(name);
},
return validValue;
}
};
/**
@@ -2117,6 +2190,22 @@ function createMenuItem(aMenu, aAttributes)
return item;
}
function throttle(func, wait, scope, window) {
var timer = null;
return function() {
if(timer) {
window.clearTimeout(timer);
}
var args = arguments;
timer = window.setTimeout(function() {
timer = null;
func.apply(scope, args);
}, wait);
};
}
/**
* Append a text node to an element.
*/

44
browser/devtools/styleinspector/test/browser_ruleview_editor_changedvalues.js
View File

@@ -70,8 +70,8 @@ function testCreateNew()
waitForEditorBlur(aEditor, function() {
promiseDone(expectRuleChange(elementRuleEditor.rule).then(() => {
is(textProp.value, "#XYZ", "Text prop should have been changed.");
is(textProp.editor._validate(), false, "#XYZ should not be a valid entry");
testEditProperty();
is(textProp.editor.isValid(), false, "#XYZ should not be a valid entry");
testCreateNewEscape();
}));
});
aEditor.input.blur();
@@ -85,6 +85,44 @@ function testCreateNew()
ruleWindow);
}
function testCreateNewEscape()
{
// Create a new property.
let elementRuleEditor = ruleView.element.children[0]._ruleEditor;
waitForEditorFocus(elementRuleEditor.element, function onNewElement(aEditor) {
is(inplaceEditor(elementRuleEditor.newPropSpan), aEditor, "Next focused editor should be the new property editor.");
let input = aEditor.input;
input.value = "color";
waitForEditorFocus(elementRuleEditor.element, function onNewValue(aEditor) {
promiseDone(expectRuleChange(elementRuleEditor.rule).then(() => {
is(elementRuleEditor.rule.textProps.length, 2, "Should have created a new text property.");
is(elementRuleEditor.propertyList.children.length, 2, "Should have created a property editor.");
let textProp = elementRuleEditor.rule.textProps[1];
is(aEditor, inplaceEditor(textProp.editor.valueSpan), "Should be editing the value span now.");
aEditor.input.value = "red";
EventUtils.synthesizeKey("VK_ESCAPE", {}, ruleWindow);
// Make sure previous input is focused.
let focusedElement = inplaceEditor(elementRuleEditor.rule.textProps[0].editor.valueSpan).input;
is(focusedElement, focusedElement.ownerDocument.activeElement, "Correct element has focus");
EventUtils.synthesizeKey("VK_ESCAPE", {}, ruleWindow);
is(elementRuleEditor.rule.textProps.length, 1, "Should have removed the new text property.");
is(elementRuleEditor.propertyList.children.length, 1, "Should have removed the property editor.");
testEditProperty();
}));
});
EventUtils.synthesizeKey("VK_RETURN", {}, ruleWindow);
});
EventUtils.synthesizeMouse(elementRuleEditor.closeBrace, 1, 1,
{ },
ruleWindow);
}
function testEditProperty()
{
let idRuleEditor = ruleView.element.children[1]._ruleEditor;
@@ -101,7 +139,7 @@ function testEditProperty()
promiseDone(expectRuleChange(idRuleEditor.rule).then(() => {
let value = idRuleEditor.rule.domRule._rawStyle().getPropertyValue("border-color");
is(value, "red", "border-color should have been set.");
is(propEditor._validate(), true, "red should be a valid entry");
is(propEditor.isValid(), true, "red should be a valid entry");
finishTest();
}));
});

19
browser/devtools/styleinspector/test/browser_ruleview_livepreview.js
View File

@@ -17,7 +17,14 @@ let inspector;
// }
let testData = [
{value: "inline", expected: "inline"},
{value: "something", expected: "inline"}
{value: "inline-block", expected: "inline-block"},
// Invalid property values should not apply, and should fall back to default
{value: "red", expected: "block"},
{value: "something", expected: "block"},
{escape: true, value: "inline", expected: "block"},
{escape: true, value: "block", expected: "block"}
];
function startTest()
@@ -49,10 +56,17 @@ function loopTestData(index)
waitForEditorFocus(propEditor.element, function(aEditor) {
is(inplaceEditor(propEditor.valueSpan), aEditor, "Focused editor should be the value.");
let thisTest = testData[index];
// Entering a correct value for the property
for (let ch of testData[index].value) {
for (let ch of thisTest.value) {
EventUtils.sendChar(ch, ruleWindow);
}
if (thisTest.escape) {
EventUtils.synthesizeKey("VK_ESCAPE", {});
} else {
EventUtils.synthesizeKey("VK_RETURN", {});
}
// While the editor is still focused in, the display should have changed already
executeSoon(() => {
@@ -60,7 +74,6 @@ function loopTestData(index)
testData[index].expected,
"Element should be previewed as " + testData[index].expected);
EventUtils.synthesizeKey("VK_RETURN", {});
loopTestData(index + 1);
});
});

2
browser/devtools/styleinspector/test/browser_ruleview_ui.js
View File

@@ -152,7 +152,7 @@ function testEditProperty()
for (let ch of "red;") {
EventUtils.sendChar(ch, ruleWindow);
is(propEditor.warning.hidden, ch == "d" || ch == ";",
is(propEditor.warning.hidden, true,
"warning triangle is hidden or shown as appropriate");
}
}));

13
toolkit/devtools/server/actors/styles.js
View File

@@ -11,6 +11,7 @@ const events = require("sdk/event/core");
const object = require("sdk/util/object");
const { Class } = require("sdk/core/heritage");
loader.lazyImporter(this, "Services", "resource://gre/modules/Services.jsm");
loader.lazyGetter(this, "CssLogic", () => require("devtools/styleinspector/css-logic").CssLogic);
loader.lazyGetter(this, "DOMUtils", () => Cc["@mozilla.org/inspector/dom-utils;1"].getService(Ci.inIDOMUtils));
@@ -710,13 +711,23 @@ var StyleRuleActor = protocol.ActorClass({
* @returns the rule with updated properties
*/
modifyProperties: method(function(modifications) {
let validProps = new Map();
// Use a fresh element for each call to this function to prevent side effects
// that pop up based on property values that were already set on the element.
let tempElement = Services.appShell.hiddenDOMWindow.
document.createElement("div");
for (let mod of modifications) {
if (mod.type === "set") {
this.rawStyle.setProperty(mod.name, mod.value, mod.priority || "");
tempElement.style.setProperty(mod.name, mod.value, mod.priority || "");
this.rawStyle.setProperty(mod.name,
tempElement.style.getPropertyValue(mod.name), mod.priority || "");
} else if (mod.type === "remove") {
this.rawStyle.removeProperty(mod.name);
}
}
return this;
}, {
request: { modifications: Arg(0, "array:json") },