Bug 1188125 - [Rule View] Properties marked overridden does not take into account disabled properties r=bgrins

browser/devtools/styleinspector/rule-view.js

@@ -334,7 +334,11 @@ ElementStyle.prototype = {
     let textProps = [];
     for (let rule of this.rules) {
       if (rule.pseudoElement == pseudo && !rule.keyframes) {
-        textProps = textProps.concat(rule.textProps.slice(0).reverse());
+        for (let textProp of rule.textProps.slice(0).reverse()) {
+          if (textProp.enabled) {
+            textProps.push(textProp);
+          }
+        }
       }
     }
 

browser/devtools/styleinspector/test/browser.ini

@@ -112,6 +112,11 @@ skip-if = (os == "win" && debug) || e10s # bug 963492: win. bug 1040653: e10s.
 [browser_ruleview_keyframes-rule_01.js]
 [browser_ruleview_keyframes-rule_02.js]
 [browser_ruleview_livepreview.js]
+[browser_ruleview_mark_overridden_01.js]
+[browser_ruleview_mark_overridden_02.js]
+[browser_ruleview_mark_overridden_03.js]
+[browser_ruleview_mark_overridden_04.js]
+[browser_ruleview_mark_overridden_05.js]
 [browser_ruleview_mathml-element.js]
 [browser_ruleview_media-queries.js]
 [browser_ruleview_multiple-properties-duplicates.js]
@@ -122,7 +127,6 @@ skip-if = (os == "win" && debug) || e10s # bug 963492: win. bug 1040653: e10s.
 [browser_ruleview_multiple_properties_02.js]
 [browser_ruleview_original-source-link.js]
 [browser_ruleview_cycle-color.js]
-[browser_ruleview_override.js]
 [browser_ruleview_pseudo-element_01.js]
 [browser_ruleview_pseudo-element_02.js]
 skip-if = e10s # Bug 1090340

browser/devtools/styleinspector/test/browser_ruleview_mark_overridden_01.js

@@ -0,0 +1,64 @@
+/* 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 the rule view marks overridden rules correctly based on the
+// specificity of the rule
+
+let TEST_URI = [
+  "<style type='text/css'>",
+  "#testid {",
+  "  background-color: blue;",
+  "}",
+  ".testclass {",
+  "  background-color: green;",
+  "}",
+  "</style>",
+  "<div id='testid' class='testclass'>Styled Node</div>"
+].join("\n");
+
+add_task(function*() {
+  yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
+  let {inspector, view} = yield openRuleView();
+  yield selectNode("#testid", inspector);
+  yield testMarkOverridden(inspector, view);
+});
+
+function* testMarkOverridden(inspector, view) {
+  let elementStyle = view._elementStyle;
+
+  let idRule = elementStyle.rules[1];
+  let idProp = idRule.textProps[0];
+  is(idProp.name, "background-color",
+    "First ID property should be background-color");
+  is(idProp.value, "blue", "First ID property value should be blue");
+  ok(!idProp.overridden, "ID prop should not be overridden.");
+  ok(!idProp.editor.element.classList.contains("ruleview-overridden"),
+    "ID property editor should not have ruleview-overridden class");
+
+  let classRule = elementStyle.rules[2];
+  let classProp = classRule.textProps[0];
+  is(classProp.name, "background-color",
+    "First class prop should be background-color");
+  is(classProp.value, "green", "First class property value should be green");
+  ok(classProp.overridden, "Class property should be overridden.");
+  ok(classProp.editor.element.classList.contains("ruleview-overridden"),
+    "Class property editor should have ruleview-overridden class");
+
+  // Override background-color by changing the element style.
+  let elementRule = elementStyle.rules[0];
+  elementRule.createProperty("background-color", "purple", "");
+  yield elementRule._applyingModifications;
+
+  let elementProp = elementRule.textProps[0];
+  ok(!elementProp.overridden,
+    "Element style property should not be overridden");
+  ok(idProp.overridden, "ID property should be overridden");
+  ok(idProp.editor.element.classList.contains("ruleview-overridden"),
+    "ID property editor should have ruleview-overridden class");
+  ok(classProp.overridden, "Class property should be overridden");
+  ok(classProp.editor.element.classList.contains("ruleview-overridden"),
+    "Class property editor should have ruleview-overridden class");
+}

browser/devtools/styleinspector/test/browser_ruleview_mark_overridden_02.js

@@ -0,0 +1,45 @@
+/* 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 the rule view marks overridden rules correctly for short hand
+// properties and the computed list properties
+
+let TEST_URI = [
+  "<style type='text/css'>",
+  "#testid {",
+  "  margin-left: 1px;",
+  "}",
+  ".testclass {",
+  "  margin: 2px;",
+  "}",
+  "</style>",
+  "<div id='testid' class='testclass'>Styled Node</div>"
+].join("\n");
+
+add_task(function*() {
+  yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
+  let {inspector, view} = yield openRuleView();
+  yield selectNode("#testid", inspector);
+  yield testMarkOverridden(inspector, view);
+});
+
+function* testMarkOverridden(inspector, view) {
+  let elementStyle = view._elementStyle;
+
+  let classRule = elementStyle.rules[2];
+  let classProp = classRule.textProps[0];
+  ok(!classProp.overridden,
+    "Class prop shouldn't be overridden, some props are still being used.");
+
+  for (let computed of classProp.computed) {
+    if (computed.name.indexOf("margin-left") == 0) {
+      ok(computed.overridden, "margin-left props should be overridden.");
+    } else {
+      ok(!computed.overridden,
+        "Non-margin-left props should not be overridden.");
+    }
+  }
+}

browser/devtools/styleinspector/test/browser_ruleview_mark_overridden_03.js

@@ -0,0 +1,48 @@
+/* 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 the rule view marks overridden rules correctly based on the
+// priority for the rule
+
+let TEST_URI = [
+  "<style type='text/css'>",
+  "#testid {",
+  "  background-color: blue;",
+  "}",
+  ".testclass {",
+  "  background-color: green !important;",
+  "}",
+  "</style>",
+  "<div id='testid' class='testclass'>Styled Node</div>"
+].join("\n");
+
+add_task(function*() {
+  yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
+  let {inspector, view} = yield openRuleView();
+  yield selectNode("#testid", inspector);
+  yield testMarkOverridden(inspector, view);
+});
+
+function* testMarkOverridden(inspector, view) {
+  let elementStyle = view._elementStyle;
+
+  let idRule = elementStyle.rules[1];
+  let idProp = idRule.textProps[0];
+  ok(idProp.overridden, "Not-important rule should be overridden.");
+
+  let classRule = elementStyle.rules[2];
+  let classProp = classRule.textProps[0];
+  ok(!classProp.overridden, "Important rule should not be overridden.");
+
+  let elementRule = elementStyle.rules[0];
+  let elementProp = elementRule.createProperty("background-color", "purple",
+    "important");
+  yield elementRule._applyingModifications;
+
+  ok(!elementProp.overridden, "New important prop should not be overriden.");
+  ok(idProp.overridden, "ID property should be overridden.");
+  ok(classProp.overridden, "Class property should be overridden.");
+}

browser/devtools/styleinspector/test/browser_ruleview_mark_overridden_04.js

@@ -0,0 +1,42 @@
+/* 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 the rule view marks overridden rules correctly if a property gets
+// disabled
+
+let TEST_URI = [
+  "<style type='text/css'>",
+  "#testid {",
+  "  background-color: blue;",
+  "}",
+  ".testclass {",
+  "  background-color: green;",
+  "}",
+  "</style>",
+  "<div id='testid' class='testclass'>Styled Node</div>"
+].join("\n");
+
+add_task(function*() {
+  yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
+  let {inspector, view} = yield openRuleView();
+  yield selectNode("#testid", inspector);
+  yield testMarkOverridden(inspector, view);
+});
+
+function* testMarkOverridden(inspector, view) {
+  let elementStyle = view._elementStyle;
+
+  let idRule = elementStyle.rules[1];
+  let idProp = idRule.textProps[0];
+
+  idProp.setEnabled(false);
+  yield idRule._applyingModifications;
+
+  let classRule = elementStyle.rules[2];
+  let classProp = classRule.textProps[0];
+  ok(!classProp.overridden,
+    "Class prop should not be overridden after id prop was disabled.");
+}

browser/devtools/styleinspector/test/browser_ruleview_mark_overridden_05.js

@@ -0,0 +1,36 @@
+/* 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 the rule view marks overridden rules correctly based on the
+// order of the property
+
+let TEST_URI = [
+  "<style type='text/css'>",
+  "#testid {",
+  "  background-color: green;",
+  "}",
+  "</style>",
+  "<div id='testid' class='testclass'>Styled Node</div>"
+].join("\n");
+
+add_task(function*() {
+  yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
+  let {inspector, view} = yield openRuleView();
+  yield selectNode("#testid", inspector);
+  yield testMarkOverridden(inspector, view);
+});
+
+function* testMarkOverridden(inspector, view) {
+  let ruleEditor = getRuleViewRuleEditor(view, 1);
+
+  yield createNewRuleViewProperty(ruleEditor, "background-color: red;");
+
+  let firstProp = ruleEditor.rule.textProps[0];
+  let secondProp = ruleEditor.rule.textProps[1];
+
+  ok(firstProp.overridden, "First property should be overridden.");
+  ok(!secondProp.overridden, "Second property should not be overridden.");
+}

browser/devtools/styleinspector/test/browser_ruleview_override.js

@@ -1,151 +0,0 @@
-/* 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 the display of overridden declarations in the rule-view
-
-add_task(function*() {
-  yield addTab("data:text/html;charset=utf-8,browser_ruleview_override.js");
-  let {toolbox, inspector, view} = yield openRuleView();
-
-  yield simpleOverride(inspector, view);
-  yield partialOverride(inspector, view);
-  yield importantOverride(inspector, view);
-  yield disableOverride(inspector, view);
-});
-
-function* createTestContent(inspector, style) {
-  let onMutated = inspector.once("markupmutation");
-  let styleNode = addStyle(content.document, style);
-  content.document.body.innerHTML = '<div id="testid" class="testclass">Styled Node</div>';
-  yield onMutated;
-  yield selectNode("#testid", inspector);
-  return styleNode;
-}
-
-function* removeTestContent(inspector, node) {
-  let onMutated = inspector.once("markupmutation");
-  node.remove();
-  yield onMutated;
-}
-
-function* simpleOverride(inspector, view) {
-  let styleNode = yield createTestContent(inspector, '' +
-    '#testid {' +
-    '  background-color: blue;' +
-    '} ' +
-    '.testclass {' +
-    '  background-color: green;' +
-    '}');
-
-  let elementStyle = view._elementStyle;
-
-  let idRule = elementStyle.rules[1];
-  let idProp = idRule.textProps[0];
-  is(idProp.name, "background-color", "First ID prop should be background-color");
-  ok(!idProp.overridden, "ID prop should not be overridden.");
-
-  let classRule = elementStyle.rules[2];
-  let classProp = classRule.textProps[0];
-  is(classProp.name, "background-color", "First class prop should be background-color");
-  ok(classProp.overridden, "Class property should be overridden.");
-
-  // Override background-color by changing the element style.
-  let elementRule = elementStyle.rules[0];
-  elementRule.createProperty("background-color", "purple", "");
-  yield elementRule._applyingModifications;
-
-  let elementProp = elementRule.textProps[0];
-  is(classProp.name, "background-color", "First element prop should now be background-color");
-  ok(!elementProp.overridden, "Element style property should not be overridden");
-  ok(idProp.overridden, "ID property should be overridden");
-  ok(classProp.overridden, "Class property should be overridden");
-
-  yield removeTestContent(inspector, styleNode);
-}
-
-function* partialOverride(inspector, view) {
-  let styleNode = yield createTestContent(inspector, '' +
-    // Margin shorthand property...
-    '.testclass {' +
-    '  margin: 2px;' +
-    '}' +
-    // ... will be partially overridden.
-    '#testid {' +
-    '  margin-left: 1px;' +
-    '}');
-
-  let elementStyle = view._elementStyle;
-
-  let classRule = elementStyle.rules[2];
-  let classProp = classRule.textProps[0];
-  ok(!classProp.overridden,
-    "Class prop shouldn't be overridden, some props are still being used.");
-
-  for (let computed of classProp.computed) {
-    if (computed.name.indexOf("margin-left") == 0) {
-      ok(computed.overridden, "margin-left props should be overridden.");
-    } else {
-      ok(!computed.overridden, "Non-margin-left props should not be overridden.");
-    }
-  }
-
-  yield removeTestContent(inspector, styleNode);
-}
-
-function* importantOverride(inspector, view) {
-  let styleNode = yield createTestContent(inspector, '' +
-    // Margin shorthand property...
-    '.testclass {' +
-    '  background-color: green !important;' +
-    '}' +
-    // ... will be partially overridden.
-    '#testid {' +
-    '  background-color: blue;' +
-    '}');
-
-  let elementStyle = view._elementStyle;
-
-  let idRule = elementStyle.rules[1];
-  let idProp = idRule.textProps[0];
-  ok(idProp.overridden, "Not-important rule should be overridden.");
-
-  let classRule = elementStyle.rules[2];
-  let classProp = classRule.textProps[0];
-  ok(!classProp.overridden, "Important rule should not be overridden.");
-
-  yield removeTestContent(inspector, styleNode);
-
-  let elementRule = elementStyle.rules[0];
-  let elementProp = elementRule.createProperty("background-color", "purple", "important");
-  yield elementRule._applyingModifications;
-
-  ok(classProp.overridden, "New important prop should override class property.");
-  ok(!elementProp.overridden, "New important prop should not be overriden.");
-}
-
-function* disableOverride(inspector, view) {
-  let styleNode = yield createTestContent(inspector, '' +
-    '#testid {' +
-    '  background-color: blue;' +
-    '}' +
-    '.testclass {' +
-    '  background-color: green;' +
-    '}');
-
-  let elementStyle = view._elementStyle;
-
-  let idRule = elementStyle.rules[1];
-  let idProp = idRule.textProps[0];
-
-  idProp.setEnabled(false);
-  yield idRule._applyingModifications;
-
-  let classRule = elementStyle.rules[2];
-  let classProp = classRule.textProps[0];
-  ok(!classProp.overridden, "Class prop should not be overridden after id prop was disabled.");
-
-  yield removeTestContent(inspector, styleNode);
-}

browser/devtools/styleinspector/test/head.js

@@ -787,11 +787,11 @@ function getRuleViewRuleEditor(view, childrenIndex, nodeIndex) {
 let focusNewRuleViewProperty = Task.async(function*(ruleEditor) {
   info("Clicking on a close ruleEditor brace to start editing a new property");
   ruleEditor.closeBrace.scrollIntoView();
-  let editor = yield focusEditableField(ruleEditor.ruleView, ruleEditor.closeBrace);
+  let editor = yield focusEditableField(ruleEditor.ruleView,
+    ruleEditor.closeBrace);
 
-  is(inplaceEditor(ruleEditor.newPropSpan), editor, "Focused editor is the new property editor.");
-  is(ruleEditor.rule.textProps.length,  0, "Starting with one new text property.");
-  is(ruleEditor.propertyList.children.length, 1, "Starting with two property editors.");
+  is(inplaceEditor(ruleEditor.newPropSpan), editor,
+    "Focused editor is the new property editor.");
 
   return editor;
 });