Bug 1253869 - Don't assume that rule in the Rules view match the selected node when checking for overidden properties. r=gl
Unmatched rules created via the "Add New Rule" button was wrongfully make other rules look not applied.
Fix returned isMatching boolean in PageStyleActor's function when the evaluated rule has multiple selectors.
Add CSS rules to make look unmatched rules' properties unmatched, like the rule's selector.
Add tests to make sur we handle unmatch rule selectors right.
MozReview-Commit-ID: FPQ7XJoa7Ba
--- a/devtools/client/inspector/rules/models/element-style.js
+++ b/devtools/client/inspector/rules/models/element-style.js
@@ -5,16 +5,17 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
"use strict";
const {Cc, Ci, Cu} = require("chrome");
const promise = require("promise");
const {Rule} = require("devtools/client/inspector/rules/models/rule");
const {promiseWarn} = require("devtools/client/inspector/shared/utils");
+const {ELEMENT_STYLE} = require("devtools/server/actors/styles");
Cu.import("resource://gre/modules/XPCOMUtils.jsm");
loader.lazyGetter(this, "PSEUDO_ELEMENTS", () => {
return domUtils.getCSSPseudoElementNames();
});
XPCOMUtils.defineLazyGetter(this, "domUtils", function () {
@@ -224,17 +225,19 @@ ElementStyle.prototype = {
markOverridden: function (pseudo = "") {
// Gather all the text properties applied by these rules, ordered
// from more- to less-specific. Text properties from keyframes rule are
// excluded from being marked as overridden since a number of criteria such
// as time, and animation overlay are required to be check in order to
// determine if the property is overridden.
let textProps = [];
for (let rule of this.rules) {
- if (rule.pseudoElement === pseudo && !rule.keyframes) {
+ if ((rule.matchedSelectors.length > 0 ||
+ rule.domRule.type === ELEMENT_STYLE) &&
+ rule.pseudoElement === pseudo && !rule.keyframes) {
for (let textProp of rule.textProps.slice(0).reverse()) {
if (textProp.enabled) {
textProps.push(textProp);
}
}
}
}
--- a/devtools/client/inspector/rules/test/browser.ini
+++ b/devtools/client/inspector/rules/test/browser.ini
@@ -121,16 +121,17 @@ skip-if = os == "mac" # Bug 1245996 : cl
[browser_rules_edit-selector_01.js]
[browser_rules_edit-selector_02.js]
[browser_rules_edit-selector_03.js]
[browser_rules_edit-selector_04.js]
[browser_rules_edit-selector_05.js]
[browser_rules_edit-selector_06.js]
[browser_rules_edit-selector_07.js]
[browser_rules_edit-selector_08.js]
+[browser_rules_edit-selector_09.js]
[browser_rules_editable-field-focus_01.js]
[browser_rules_editable-field-focus_02.js]
[browser_rules_eyedropper.js]
[browser_rules_filtereditor-appears-on-swatch-click.js]
[browser_rules_filtereditor-commit-on-ENTER.js]
[browser_rules_filtereditor-revert-on-ESC.js]
skip-if = (os == "win" && debug) # bug 963492: win.
[browser_rules_guessIndentation.js]
--- a/devtools/client/inspector/rules/test/browser_rules_edit-selector_05.js
+++ b/devtools/client/inspector/rules/test/browser_rules_edit-selector_05.js
@@ -69,10 +69,10 @@ function* checkModifiedElement(view, nam
ok(getRuleViewRule(view, name), "Rule with " + name + " selector exists.");
}
function* testAddProperty(view) {
info("Test creating a new property");
let textProp = yield addProperty(view, 1, "text-align", "center");
is(textProp.value, "center", "Text prop should have been changed.");
- is(textProp.overridden, false, "Property should not be overridden");
+ ok(!textProp.overridden, "Property should not be overridden");
}
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/rules/test/browser_rules_edit-selector_09.js
@@ -0,0 +1,110 @@
+/* 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";
+
+// Tests that editing a selector to an unmatched rule does set up the correct
+// property on the rule, and that settings property in said rule does not
+// lead to overriding properties from matched rules.
+// Test that having a rule with both matched and unmatched selectors does work
+// correctly.
+
+const TEST_URI = `
+ <style type="text/css">
+ #testid {
+ color: black;
+ }
+ .testclass {
+ background-color: white;
+ }
+ </style>
+ <div id="testid">Styled Node</div>
+ <span class="testclass">This is a span</span>
+`;
+
+add_task(function* () {
+ yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
+ let {inspector, view} = yield openRuleView();
+
+ yield selectNode("#testid", inspector);
+ yield testEditSelector(view, "span");
+ yield testAddImportantProperty(view);
+ yield testAddMatchedRule(view, "span, div");
+});
+
+function* testEditSelector(view, name) {
+ info("Test editing existing selector fields");
+
+ let ruleEditor = getRuleViewRuleEditor(view, 1);
+
+ info("Focusing an existing selector name in the rule-view");
+ let editor = yield focusEditableField(view, ruleEditor.selectorText);
+
+ is(inplaceEditor(ruleEditor.selectorText), editor,
+ "The selector editor got focused");
+
+ info("Entering a new selector name and committing");
+ editor.input.value = name;
+
+ info("Waiting for rule view to update");
+ let onRuleViewChanged = once(view, "ruleview-changed");
+
+ info("Entering the commit key");
+ EventUtils.synthesizeKey("VK_RETURN", {});
+ yield onRuleViewChanged;
+
+ ok(getRuleViewRule(view, name), "Rule with " + name + " selector exists.");
+ ok(getRuleViewRuleEditor(view, 1).element.getAttribute("unmatched"),
+ "Rule with " + name + " does not match the current element.");
+
+ // Escape the new property editor after editing the selector
+ let onBlur = once(view.styleDocument.activeElement, "blur");
+ EventUtils.synthesizeKey("VK_ESCAPE", {}, view.styleWindow);
+ yield onBlur;
+}
+
+function* testAddImportantProperty(view) {
+ info("Test creating a new property with !important");
+ let textProp = yield addProperty(view, 1, "color", "red !important");
+
+ is(textProp.value, "red", "Text prop should have been changed.");
+ is(textProp.priority, "important",
+ "Text prop has an \"important\" priority.");
+ ok(!textProp.overridden, "Property should not be overridden");
+
+ let ruleEditor = getRuleViewRuleEditor(view, 1);
+ let prop = ruleEditor.rule.textProps[0];
+ ok(!prop.overridden,
+ "Existing property on matched rule should not be overridden");
+}
+
+function* testAddMatchedRule(view, name) {
+ info("Test adding a matching selector");
+
+ let ruleEditor = getRuleViewRuleEditor(view, 1);
+
+ info("Focusing an existing selector name in the rule-view");
+ let editor = yield focusEditableField(view, ruleEditor.selectorText);
+
+ is(inplaceEditor(ruleEditor.selectorText), editor,
+ "The selector editor got focused");
+
+ info("Entering a new selector name and committing");
+ editor.input.value = name;
+
+ info("Waiting for rule view to update");
+ let onRuleViewChanged = once(view, "ruleview-changed");
+
+ info("Entering the commit key");
+ EventUtils.synthesizeKey("VK_RETURN", {});
+ yield onRuleViewChanged;
+
+ is(getRuleViewRuleEditor(view, 1).element.getAttribute("unmatched"), "false",
+ "Rule with " + name + " does match the current element.");
+
+ // Escape the new property editor after editing the selector
+ let onBlur = once(view.styleDocument.activeElement, "blur");
+ EventUtils.synthesizeKey("VK_ESCAPE", {}, view.styleWindow);
+ yield onBlur;
+}
--- a/devtools/client/themes/rules.css
+++ b/devtools/client/themes/rules.css
@@ -207,16 +207,20 @@
* Display rules that don't match the current selected element and uneditable
* user agent styles differently
*/
.ruleview-rule[unmatched=true],
.ruleview-rule[uneditable=true] {
background: var(--theme-tab-toolbar-background);
}
+.ruleview-rule[unmatched=true] {
+ opacity: 0.5;
+}
+
.ruleview-rule[uneditable=true] :focus {
outline: none;
}
.ruleview-rule[uneditable=true] .theme-link {
color: var(--theme-highlight-bluegrey);
}
@@ -443,17 +447,18 @@
border-bottom-color: hsl(0,0%,50%);
}
.ruleview-selectorcontainer {
word-wrap: break-word;
cursor: text;
}
-.ruleview-selector-separator, .ruleview-selector-unmatched {
+.ruleview-selector-separator,
+.ruleview-selector-unmatched {
color: #888;
}
.ruleview-selector-matched > .ruleview-selector-attribute {
/* TODO: Bug 1178535 Awaiting UX feedback on highlight colors */
}
.ruleview-selector-matched > .ruleview-selector-pseudo-class {
--- a/devtools/server/actors/styles.js
+++ b/devtools/server/actors/styles.js
@@ -1462,23 +1462,25 @@ var StyleRuleActor = protocol.ActorClass
return selectorPromise.then((newCssRule) => {
let ruleProps = null;
let isMatching = false;
if (newCssRule) {
let ruleEntry = this.pageStyle.findEntryMatchingRule(node, newCssRule);
if (ruleEntry.length === 1) {
- isMatching = true;
ruleProps =
this.pageStyle.getAppliedProps(node, ruleEntry,
{ matchedSelectors: true });
} else {
ruleProps = this.pageStyle.getNewAppliedProps(node, newCssRule);
}
+
+ isMatching = ruleProps.entries.some((ruleProp) =>
+ ruleProp.matchedSelectors.length > 0);
}
return { ruleProps, isMatching };
});
}
});
/**