Bug 1411664 - Check if property name is invalid in rule view and change warning icon title text accordingly. r=pbro
MozReview-Commit-ID: L64Ow4cLAg8
--- a/devtools/client/inspector/rules/models/text-property.js
+++ b/devtools/client/inspector/rules/models/text-property.js
@@ -182,34 +182,50 @@ TextProperty.prototype = {
isKnownProperty: function() {
return this.cssProperties.isKnown(this.name);
},
/**
* Validate this property. Does it make sense for this value to be assigned
* to this property name?
*
- * @return {Boolean} true if the property value is valid, false otherwise.
+ * @return {Boolean} true if the whole CSS declaration is valid, false otherwise.
*/
isValid: function() {
- // Starting with FF49, StyleRuleActors provide a list of parsed
- // declarations, with data about their validity, but if we don't have this,
- // compute validity locally (which might not be correct, but better than
- // nothing).
- if (!this.rule.domRule.declarations) {
- return this.cssProperties.isValidOnClient(this.name, this.value, this.panelDoc);
- }
-
let selfIndex = this.rule.textProps.indexOf(this);
// When adding a new property in the rule-view, the TextProperty object is
// created right away before the rule gets updated on the server, so we're
// not going to find the corresponding declaration object yet. Default to
// true.
if (!this.rule.domRule.declarations[selfIndex]) {
return true;
}
return this.rule.domRule.declarations[selfIndex].isValid;
+ },
+
+ /**
+ * Validate the name of this property.
+ *
+ * @return {Boolean} true if the property name is valid, false otherwise.
+ */
+ isNameValid: function() {
+ let selfIndex = this.rule.textProps.indexOf(this);
+
+ // When adding a new property in the rule-view, the TextProperty object is
+ // created right away before the rule gets updated on the server, so we're
+ // not going to find the corresponding declaration object yet. Default to
+ // true.
+ if (!this.rule.domRule.declarations[selfIndex]) {
+ return true;
+ }
+
+ // Starting with FF61, StyleRuleActor provides an accessor to signal if the property
+ // name is valid. If we don't have this, assume the name is valid. In use, rely on
+ // isValid() as a guard against false positives.
+ return (this.rule.domRule.declarations[selfIndex].isNameValid !== undefined)
+ ? this.rule.domRule.declarations[selfIndex].isNameValid
+ : true;
}
};
module.exports = TextProperty;
--- a/devtools/client/inspector/rules/test/browser_rules_authored.js
+++ b/devtools/client/inspector/rules/test/browser_rules_authored.js
@@ -27,23 +27,28 @@ add_task(async function() {
// Override.
" background-color: blue;" +
" background-color: #f06;" +
"} ");
let elementStyle = view._elementStyle;
let expected = [
- {name: "something", overridden: true},
- {name: "color", overridden: true},
- {name: "background-color", overridden: true},
- {name: "background-color", overridden: false}
+ {name: "something", overridden: true, isNameValid: false, isValid: false},
+ {name: "color", overridden: true, isNameValid: true, isValid: false},
+ {name: "background-color", overridden: true, isNameValid: true, isValid: true},
+ {name: "background-color", overridden: false, isNameValid: true, isValid: true}
];
let rule = elementStyle.rules[1];
for (let i = 0; i < expected.length; ++i) {
let prop = rule.textProps[i];
- is(prop.name, expected[i].name, "test name for prop " + i);
+ is(prop.name, expected[i].name,
+ "Check name for prop " + i);
is(prop.overridden, expected[i].overridden,
- "test overridden for prop " + i);
+ "Check overridden for prop " + i);
+ is(prop.isNameValid(), expected[i].isNameValid,
+ "Check if property name is valid for prop " + i);
+ is(prop.isValid(), expected[i].isValid,
+ "Check if whole declaration is valid for prop " + i);
}
});
--- a/devtools/client/inspector/rules/views/text-property-editor.js
+++ b/devtools/client/inspector/rules/views/text-property-editor.js
@@ -567,16 +567,20 @@ TextPropertyEditor.prototype = {
if (this.prop.enabled) {
this.enable.style.removeProperty("visibility");
this.enable.setAttribute("checked", "");
} else {
this.enable.style.visibility = "visible";
this.enable.removeAttribute("checked");
}
+ this.warning.title = !this.isNameValid()
+ ? l10n("rule.warningName.title")
+ : l10n("rule.warning.title");
+
this.warning.hidden = this.editing || this.isValid();
this.filterProperty.hidden = this.editing ||
!this.isValid() ||
!this.prop.overridden ||
this.ruleEditor.rule.isUnmatched;
let showExpander = this.prop.computed.some(c => c.name !== this.prop.name);
this.expander.style.display = showExpander ? "inline-block" : "none";
@@ -1003,23 +1007,31 @@ TextPropertyEditor.prototype = {
this.ruleEditor.rule.previewPropertyValue(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
*
- * @return {Boolean} true if the property value is valid, false otherwise.
+ * @return {Boolean} true if the property name + value pair is valid, false otherwise.
*/
isValid: function() {
return this.prop.isValid();
},
/**
+ * Validate the name of this property.
+ * @return {Boolean} true if the property name is valid, false otherwise.
+ */
+ isNameValid: function() {
+ return this.prop.isNameValid();
+ },
+
+ /**
* Returns true if the property is a `display: [inline-]flex` declaration.
*
* @return {Boolean} true if the property is a `display: [inline-]flex` declaration.
*/
isDisplayFlex: function() {
return this.prop.name === "display" &&
(this.prop.value === "flex" || this.prop.value === "inline-flex");
},
--- a/devtools/server/actors/styles.js
+++ b/devtools/server/actors/styles.js
@@ -1124,16 +1124,18 @@ var StyleRuleActor = protocol.ActorClass
// We need to grab CSS from the window, since calling supports() on the
// one from the current global will fail due to not being an HTML global.
let CSS = this.pageStyle.inspector.tabActor.window.CSS;
form.declarations = declarations.map(decl => {
// Use the 1-arg CSS.supports() call so that we also accept !important
// in the value.
decl.isValid = CSS.supports(`${decl.name}:${decl.value}`);
+ // Check property name. All valid CSS properties support "initial" as a value.
+ decl.isNameValid = CSS.supports(decl.name, "initial");
return decl;
});
}
return form;
},
/**
--- a/devtools/shared/locales/en-US/styleinspector.properties
+++ b/devtools/shared/locales/en-US/styleinspector.properties
@@ -53,16 +53,21 @@ rule.pseudoElement=Pseudo-elements
# pseudo elements are present in the rule view.
rule.selectedElement=This Element
# LOCALIZATION NOTE (rule.warning.title): When an invalid property value is
# entered into the rule view a warning icon is displayed. This text is used for
# the title attribute of the warning icon.
rule.warning.title=Invalid property value
+# LOCALIZATION NOTE (rule.warningName.title): When an invalid property name is
+# entered into the rule view a warning icon is displayed. This text is used for
+# the title attribute of the warning icon.
+rule.warningName.title=Invalid property name
+
# LOCALIZATION NOTE (rule.filterProperty.title): Text displayed in the tooltip
# of the search button that is shown next to a property that has been overridden
# in the rule view.
rule.filterProperty.title=Filter rules containing this property
# LOCALIZATION NOTE (rule.empty): Text displayed when the highlighter is
# first opened and there's no node selected in the rule view.
rule.empty=No element selected.