Bug 1184628 - [Rule View] Editing a disabled property should enable it r=bgrins
authorGabriel Luong <gabriel.luong@gmail.com>
Mon, 27 Jul 2015 21:40:34 -0700
changeset 286493 2316676f94d54df4a7651c7ede57fa255439f23a
parent 286492 72b6857ff7840f06b1e7cc0f1f736784884cc024
child 286494 4be4031b55e3a399023cdb164f88f51534ca7542
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbgrins
bugs1184628
milestone42.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1184628 - [Rule View] Editing a disabled property should enable it r=bgrins
browser/devtools/styleinspector/rule-view.js
browser/devtools/styleinspector/test/browser.ini
browser/devtools/styleinspector/test/browser_ruleview_edit-property_04.js
browser/devtools/styleinspector/test/browser_ruleview_edit-property_05.js
browser/devtools/styleinspector/test/browser_ruleview_edit-property_06.js
browser/devtools/styleinspector/test/browser_ruleview_edit-property_07.js
--- a/browser/devtools/styleinspector/rule-view.js
+++ b/browser/devtools/styleinspector/rule-view.js
@@ -3158,17 +3158,16 @@ TextPropertyEditor.prototype = {
     // Populate the computed styles.
     this._updateComputed();
 
     // Update the rule property highlight.
     this.ruleView._updatePropertyHighlight(this);
   },
 
   _onStartEditing: function() {
-    this.element.classList.remove("ruleview-overridden");
     this._previewValue(this.prop.value);
   },
 
   /**
    * Populate the list of computed styles.
    */
   _updateComputed: function() {
     // Clear out existing viewers.
@@ -3321,16 +3320,20 @@ TextPropertyEditor.prototype = {
     // Adding multiple rules inside of name field overwrites the current
     // property with the first, then adds any more onto the property list.
     let properties = parseDeclarations(aValue);
 
     if (properties.length) {
       this.prop.setName(properties[0].name);
       this.committed.name = this.prop.name;
 
+      if (!this.prop.enabled) {
+        this.prop.setEnabled(true);
+      }
+
       if (properties.length > 1) {
         this.prop.setValue(properties[0].value, properties[0].priority);
         this.ruleEditor.addProperties(properties.slice(1), this.prop);
       }
     }
   },
 
   /**
@@ -3376,16 +3379,21 @@ TextPropertyEditor.prototype = {
         // Disable the property if the property was originally disabled.
         this.rule.setPropertyEnabled(this.prop, this.prop.enabled);
       }
       return;
     }
 
     // First, set this property value (common case, only modified a property)
     this.prop.setValue(val.value, val.priority);
+
+    if (!this.prop.enabled) {
+      this.prop.setEnabled(true);
+    }
+
     this.removeOnRevert = false;
     this.committed.value = this.prop.value;
     this.committed.priority = this.prop.priority;
 
     // If needed, add any new properties after this.prop.
     this.ruleEditor.addProperties(parsedProperties.propertiesToAdd, this.prop);
 
     // If the name or value is not actively being edited, and the value is
@@ -3455,16 +3463,19 @@ TextPropertyEditor.prototype = {
    */
   _previewValue: function(aValue) {
     // Since function call is throttled, we need to make sure we are still
     // editing, and any selector modifications have been completed
     if (!this.editing || this.ruleEditor.isEditing) {
       return;
     }
 
+    this.element.classList.remove("ruleview-overridden");
+    this.enable.style.visibility = "hidden";
+
     let val = parseSingleValue(aValue);
     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
--- a/browser/devtools/styleinspector/test/browser.ini
+++ b/browser/devtools/styleinspector/test/browser.ini
@@ -88,16 +88,19 @@ skip-if = e10s # Bug 1039528: "inspect e
 [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]
 [browser_ruleview_edit-property_02.js]
 [browser_ruleview_edit-property_03.js]
 [browser_ruleview_edit-property_04.js]
+[browser_ruleview_edit-property_05.js]
+[browser_ruleview_edit-property_06.js]
+[browser_ruleview_edit-property_07.js]
 [browser_ruleview_edit-selector-commit.js]
 [browser_ruleview_edit-selector_01.js]
 [browser_ruleview_edit-selector_02.js]
 [browser_ruleview_edit-selector_03.js]
 [browser_ruleview_edit-selector_04.js]
 [browser_ruleview_edit-selector_05.js]
 [browser_ruleview_eyedropper.js]
 [browser_ruleview_filtereditor-appears-on-swatch-click.js]
--- a/browser/devtools/styleinspector/test/browser_ruleview_edit-property_04.js
+++ b/browser/devtools/styleinspector/test/browser_ruleview_edit-property_04.js
@@ -34,41 +34,55 @@ function* testDisableProperty(inspector,
 
   let newValue = yield executeInContent("Test:GetRulePropertyValue", {
     styleSheetIndex: 0,
     ruleIndex: 0,
     name: "background-color"
   });
   is(newValue, "", "background-color should have been unset.");
 
-  yield testPreviewDisableProperty(view, ruleEditor, propEditor.nameSpan,
-    "VK_ESCAPE");
-  yield testPreviewDisableProperty(view, ruleEditor, propEditor.valueSpan,
-    "VK_ESCAPE");
-  yield testPreviewDisableProperty(view, ruleEditor, propEditor.valueSpan,
-    "VK_TAB");
-  yield testPreviewDisableProperty(view, ruleEditor, propEditor.valueSpan,
-    "VK_RETURN");
+  yield testPreviewDisableProperty(view, ruleEditor, propEditor,
+    propEditor.nameSpan, "VK_ESCAPE");
+  yield testPreviewDisableProperty(view, ruleEditor, propEditor,
+    propEditor.valueSpan, "VK_ESCAPE");
+  yield testPreviewDisableProperty(view, ruleEditor, propEditor,
+    propEditor.valueSpan, "VK_TAB");
+  yield testPreviewDisableProperty(view, ruleEditor, propEditor,
+    propEditor.valueSpan, "VK_RETURN");
 }
 
-function* testPreviewDisableProperty(view, ruleEditor, propEditor, commitKey) {
-  let editor = yield focusEditableField(view, propEditor);
+function* testPreviewDisableProperty(view, ruleEditor, propEditor,
+    editableField, commitKey) {
+  let editor = yield focusEditableField(view, editableField);
   yield ruleEditor.rule._applyingModifications;
 
+  ok(!propEditor.element.classList.contains("ruleview-overridden"),
+    "property is not overridden.");
+  is(propEditor.enable.style.visibility, "hidden",
+    "property enable checkbox is hidden.");
+
   let newValue = yield executeInContent("Test:GetRulePropertyValue", {
     styleSheetIndex: 0,
     ruleIndex: 0,
     name: "background-color"
   });
   is(newValue, "blue", "background-color should have been previewed.");
 
   let onBlur = once(editor.input, "blur");
   EventUtils.synthesizeKey(commitKey, {}, view.styleWindow);
   yield onBlur;
   yield ruleEditor.rule._applyingModifications;
 
+  ok(!propEditor.prop.enabled, "property is disabled.");
+  ok(propEditor.element.classList.contains("ruleview-overridden"),
+    "property is overridden.");
+  is(propEditor.enable.style.visibility, "visible",
+    "property enable checkbox is visible.");
+  ok(!propEditor.enable.getAttribute("checked"),
+    "property enable checkbox is not checked.");
+
   newValue = yield executeInContent("Test:GetRulePropertyValue", {
     styleSheetIndex: 0,
     ruleIndex: 0,
     name: "background-color"
   });
   is(newValue, "", "background-color should have been unset.");
 }
new file mode 100644
--- /dev/null
+++ b/browser/devtools/styleinspector/test/browser_ruleview_edit-property_05.js
@@ -0,0 +1,86 @@
+/* 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 a disabled property is re-enabled if the property name or value is
+// modified
+
+let TEST_URI = [
+  "<style type='text/css'>",
+  "#testid {",
+  "  background-color: blue;",
+  "}",
+  "</style>",
+  "<div id='testid'>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 testEditingDisableProperty(inspector, view);
+});
+
+function* testEditingDisableProperty(inspector, view) {
+  let ruleEditor = getRuleViewRuleEditor(view, 1);
+  let propEditor = ruleEditor.rule.textProps[0].editor;
+
+  info("Disabling background-color property");
+  propEditor.enable.click();
+  yield ruleEditor.rule._applyingModifications;
+
+  let newValue = yield getRulePropertyValue("background-color");
+  is(newValue, "", "background-color should have been unset.");
+
+  yield focusEditableField(view, propEditor.nameSpan);
+
+  info("Entering a new property name, including : to commit and " +
+       "focus the value");
+  let onValueFocus = once(ruleEditor.element, "focus", true);
+  EventUtils.sendString("border-color:", view.styleWindow);
+  yield onValueFocus;
+  yield ruleEditor.rule._applyingModifications;
+
+  info("Escape editing the property value");
+  EventUtils.synthesizeKey("VK_ESCAPE", {}, view.styleWindow);
+  yield ruleEditor.rule._applyingModifications;
+
+  newValue = yield getRulePropertyValue("border-color");
+  is(newValue, "blue", "border-color should have been set.");
+
+  ok(propEditor.prop.enabled, "border-color property is enabled.");
+  ok(!propEditor.element.classList.contains("ruleview-overridden"),
+    "border-color is not overridden");
+
+  info("Disabling border-color property");
+  propEditor.enable.click();
+  yield ruleEditor.rule._applyingModifications;
+
+  newValue = yield getRulePropertyValue("border-color");
+  is(newValue, "", "border-color should have been unset.");
+
+  info("Enter a new property value for the border-color property");
+  let editor = yield focusEditableField(view, propEditor.valueSpan);
+  let onBlur = once(editor.input, "blur");
+  EventUtils.sendString("red;", view.styleWindow);
+  yield onBlur;
+  yield ruleEditor.rule._applyingModifications;
+
+  newValue = yield getRulePropertyValue("border-color");
+  is(newValue, "red", "new border-color should have been set.");
+
+  ok(propEditor.prop.enabled, "border-color property is enabled.");
+  ok(!propEditor.element.classList.contains("ruleview-overridden"),
+    "border-color is not overridden");
+}
+
+function* getRulePropertyValue(name) {
+  let propValue = yield executeInContent("Test:GetRulePropertyValue", {
+    styleSheetIndex: 0,
+    ruleIndex: 0,
+    name: name
+  });
+  return propValue;
+}
new file mode 100644
--- /dev/null
+++ b/browser/devtools/styleinspector/test/browser_ruleview_edit-property_06.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 editing a property's priority is behaving correctly, and disabling
+// and editing the property will re-enable the property.
+
+let TEST_URI = [
+  "<style type='text/css'>",
+  "body {",
+  "  background-color: green !important;",
+  "}",
+  "body {",
+  "  background-color: red;",
+  "}",
+  "</style>",
+].join("\n");
+
+add_task(function*() {
+  yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
+  let {inspector, view} = yield openRuleView();
+  yield selectNode("body", inspector);
+  yield testEditPropertyPriorityAndDisable(inspector, view);
+});
+
+function* testEditPropertyPriorityAndDisable(inspector, view) {
+  let ruleEditor = getRuleViewRuleEditor(view, 1);
+  let propEditor = ruleEditor.rule.textProps[0].editor;
+
+  is((yield getComputedStyleProperty("body", null, "background-color")),
+    "rgb(0, 128, 0)", "green background color is set.");
+
+  let editor = yield focusEditableField(view, propEditor.valueSpan);
+  let onBlur = once(editor.input, "blur");
+  EventUtils.sendString("red !important;", view.styleWindow);
+  yield onBlur;
+  yield ruleEditor.rule._applyingModifications;
+
+  is(propEditor.valueSpan.textContent, "red !important",
+    "'red !important' property value is correctly set.");
+  is((yield getComputedStyleProperty("body", null, "background-color")),
+    "rgb(255, 0, 0)", "red background color is set.");
+
+  info("Disabling red background color property");
+  propEditor.enable.click();
+  yield ruleEditor.rule._applyingModifications;
+
+  is((yield getComputedStyleProperty("body", null, "background-color")),
+    "rgb(0, 128, 0)", "green background color is set.");
+
+  editor = yield focusEditableField(view, propEditor.valueSpan);
+  onBlur = once(editor.input, "blur");
+  EventUtils.sendString("red;", view.styleWindow);
+  yield onBlur;
+  yield ruleEditor.rule._applyingModifications;
+
+  is(propEditor.valueSpan.textContent, "red",
+    "'red' property value is correctly set.");
+  ok(propEditor.prop.enabled, "red background-color property is enabled.");
+  is((yield getComputedStyleProperty("body", null, "background-color")),
+    "rgb(0, 128, 0)", "green background color is set.");
+}
new file mode 100644
--- /dev/null
+++ b/browser/devtools/styleinspector/test/browser_ruleview_edit-property_07.js
@@ -0,0 +1,55 @@
+/* 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 adding multiple values will enable the property even if the
+// property does not change, and that the extra values are added correctly.
+
+let TEST_URI = [
+  "<style type='text/css'>",
+  "#testid {",
+  "  background-color: red;",
+  "}",
+  "</style>",
+  "<div id='testid'>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 testEditDisableProperty(inspector, view);
+});
+
+function* testEditDisableProperty(inspector, view) {
+  let ruleEditor = getRuleViewRuleEditor(view, 1);
+  let propEditor = ruleEditor.rule.textProps[0].editor;
+
+  info("Disabling red background color property");
+  propEditor.enable.click();
+  yield ruleEditor.rule._applyingModifications;
+
+  ok(!propEditor.prop.enabled, "red background-color property is disabled.");
+
+  let editor = yield focusEditableField(view, propEditor.valueSpan);
+  let onBlur = once(editor.input, "blur");
+  EventUtils.sendString("red; color: red;", view.styleWindow);
+  yield onBlur;
+  yield ruleEditor.rule._applyingModifications;
+
+  is(propEditor.valueSpan.textContent, "red",
+    "'red' property value is correctly set.");
+  ok(propEditor.prop.enabled, "red background-color property is enabled.");
+  is((yield getComputedStyleProperty("#testid", null, "background-color")),
+    "rgb(255, 0, 0)", "red background color is set.");
+
+  propEditor = ruleEditor.rule.textProps[1].editor;
+  is(propEditor.nameSpan.textContent, "color",
+    "new 'color' property name is correctly set.");
+  is(propEditor.valueSpan.textContent, "red",
+    "new 'red' property value is correctly set.");
+  is((yield getComputedStyleProperty("#testid", null, "color")),
+    "rgb(255, 0, 0)", "red color is set.");
+}