Bug 1293616 - insert new rule in correct position in rule list; r=pbro

MozReview-Commit-ID: C4ib7ooI8BX

--HG--
extra : rebase_source : c6da75c8cd688b80929eaa1365833310dd61bcf0

devtools/client/inspector/rules/test/browser.ini

@@ -128,6 +128,8 @@ skip-if = os == "mac" # Bug 1245996 : click on scrollbar not working on OSX
 [browser_rules_edit-selector_07.js]
 [browser_rules_edit-selector_08.js]
 [browser_rules_edit-selector_09.js]
+[browser_rules_edit-selector_10.js]
+[browser_rules_edit-selector_11.js]
 [browser_rules_edit-value-after-name_01.js]
 [browser_rules_edit-value-after-name_02.js]
 [browser_rules_edit-value-after-name_03.js]

devtools/client/inspector/rules/test/browser_rules_edit-selector_10.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";
+
+// Regression test for bug 1293616: make sure that editing a selector
+// keeps the rule in the proper position.
+
+const TEST_URI = `
+  <style type="text/css">
+    #testid span, #testid p {
+      background: aqua;
+    }
+    span {
+      background: fuchsia;
+    }
+  </style>
+  <div id="testid">
+    <span class="pickme">
+      Styled Node
+    </span>
+  </div>
+`;
+
+add_task(function* () {
+  yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
+  let {inspector, view} = yield openRuleView();
+  yield selectNode(".pickme", inspector);
+  yield testEditSelector(view);
+});
+
+function* testEditSelector(view) {
+  let ruleEditor = getRuleViewRuleEditor(view, 1);
+  let editor = yield focusEditableField(view, ruleEditor.selectorText);
+
+  editor.input.value = "#testid span";
+  let onRuleViewChanged = once(view, "ruleview-changed");
+  EventUtils.synthesizeKey("VK_RETURN", {});
+  yield onRuleViewChanged;
+
+  // Escape the new property editor after editing the selector
+  let onBlur = once(view.styleDocument.activeElement, "blur");
+  EventUtils.synthesizeKey("VK_ESCAPE", {}, view.styleWindow);
+  yield onBlur;
+
+  // Get the new rule editor that replaced the original
+  ruleEditor = getRuleViewRuleEditor(view, 1);
+
+  info("Check that the correct rules are visible");
+  is(view._elementStyle.rules.length, 3, "Should have 3 rules.");
+  is(ruleEditor.element.getAttribute("unmatched"), "false", "Rule editor is matched.");
+
+  let props = ruleEditor.rule.textProps;
+  is(props.length, 1, "Rule has correct number of properties");
+  is(props[0].name, "background", "Found background property");
+  ok(!props[0].overridden, "Background property is not overridden");
+
+  ruleEditor = getRuleViewRuleEditor(view, 2);
+  props = ruleEditor.rule.textProps;
+  is(props.length, 1, "Rule has correct number of properties");
+  is(props[0].name, "background", "Found background property");
+  ok(props[0].overridden, "Background property is overridden");
+}

devtools/client/inspector/rules/test/browser_rules_edit-selector_11.js

@@ -0,0 +1,69 @@
+/* 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";
+
+// Regression test for bug 1293616, where editing a selector should
+// change the relative priority of the rule.
+
+const TEST_URI = `
+  <style type="text/css">
+    #testid {
+      background: aqua;
+    }
+    .pickme {
+      background: seagreen;
+    }
+    span {
+      background: fuchsia;
+    }
+  </style>
+  <div>
+    <span id="testid" class="pickme">
+      Styled Node
+    </span>
+  </div>
+`;
+
+add_task(function* () {
+  yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
+  let {inspector, view} = yield openRuleView();
+  yield selectNode(".pickme", inspector);
+  yield testEditSelector(view);
+});
+
+function* testEditSelector(view) {
+  let ruleEditor = getRuleViewRuleEditor(view, 1);
+  let editor = yield focusEditableField(view, ruleEditor.selectorText);
+
+  editor.input.value = ".pickme";
+  let onRuleViewChanged = once(view, "ruleview-changed");
+  EventUtils.synthesizeKey("VK_RETURN", {});
+  yield onRuleViewChanged;
+
+  // Escape the new property editor after editing the selector
+  let onBlur = once(view.styleDocument.activeElement, "blur");
+  EventUtils.synthesizeKey("VK_ESCAPE", {}, view.styleWindow);
+  yield onBlur;
+
+  // Get the new rule editor that replaced the original
+  ruleEditor = getRuleViewRuleEditor(view, 1);
+
+  info("Check that the correct rules are visible");
+  is(view._elementStyle.rules.length, 4, "Should have 4 rules.");
+  is(ruleEditor.element.getAttribute("unmatched"), "false", "Rule editor is matched.");
+
+  let props = ruleEditor.rule.textProps;
+  is(props.length, 1, "Rule has correct number of properties");
+  is(props[0].name, "background", "Found background property");
+  is(props[0].value, "aqua", "Background property is aqua");
+  ok(props[0].overridden, "Background property is overridden");
+
+  ruleEditor = getRuleViewRuleEditor(view, 2);
+  props = ruleEditor.rule.textProps;
+  is(props.length, 1, "Rule has correct number of properties");
+  is(props[0].name, "background", "Found background property");
+  is(props[0].value, "seagreen", "Background property is seagreen");
+  ok(!props[0].overridden, "Background property is not overridden");
+}

devtools/client/inspector/rules/views/rule-editor.js

@@ -27,6 +27,7 @@ const {
 const promise = require("promise");
 const Services = require("Services");
 const EventEmitter = require("devtools/shared/event-emitter");
+const {Task} = require("devtools/shared/task");
 
 const STYLE_INSPECTOR_PROPERTIES = "devtools-shared/locale/styleinspector.properties";
 const {LocalizationHelper} = require("devtools/shared/l10n");
@@ -517,7 +518,7 @@ RuleEditor.prototype = {
    * @param {Number} direction
    *        The move focus direction number.
    */
-  _onSelectorDone: function (value, commit, direction) {
+  _onSelectorDone: Task.async(function* (value, commit, direction) {
     if (!commit || this.isEditing || value === "" ||
         value === this.rule.selectorText) {
       return;
@@ -531,16 +532,28 @@ RuleEditor.prototype = {
 
     this.isEditing = true;
 
-    this.rule.domRule.modifySelector(element, value).then(response => {
-      this.isEditing = false;
+    try {
+      let response = yield this.rule.domRule.modifySelector(element, value);
 
       if (!supportsUnmatchedRules) {
+        this.isEditing = false;
+
         if (response) {
           this.ruleView.refreshPanel();
         }
         return;
       }
 
+      // We recompute the list of applied styles, because editing a
+      // selector might cause this rule's position to change.
+      let applied = yield elementStyle.pageStyle.getApplied(element, {
+        inherited: true,
+        matchedSelectors: true,
+        filter: elementStyle.showUserAgentStyles ? "ua" : undefined
+      });
+
+      this.isEditing = false;
+
       let {ruleProps, isMatching} = response;
       if (!ruleProps) {
         // Notify for changes, even when nothing changes,
@@ -554,11 +567,25 @@ RuleEditor.prototype = {
       let editor = new RuleEditor(ruleView, newRule);
       let rules = elementStyle.rules;
 
-      rules.splice(rules.indexOf(this.rule), 1);
-      rules.push(newRule);
+      let newRuleIndex = applied.findIndex((r) => r.rule == ruleProps.rule);
+      let oldIndex = rules.indexOf(this.rule);
+
+      // If the selector no longer matches, then we leave the rule in
+      // the same relative position.
+      if (newRuleIndex === -1) {
+        newRuleIndex = oldIndex;
+      }
+
+      // Remove the old rule and insert the new rule.
+      rules.splice(oldIndex, 1);
+      rules.splice(newRuleIndex, 0, newRule);
       elementStyle._changed();
       elementStyle.markOverriddenAll();
 
+      // We install the new editor in place of the old -- you might
+      // think we would replicate the list-modification logic above,
+      // but that is complicated due to the way the UI installs
+      // pseudo-element rules and the like.
       this.element.parentNode.replaceChild(editor.element, this.element);
 
       // Remove highlight for modified selector
@@ -568,11 +595,11 @@ RuleEditor.prototype = {
       }
 
       editor._moveSelectorFocus(direction);
-    }).then(null, err => {
+    } catch (err) {
       this.isEditing = false;
       promiseWarn(err);
-    });
-  },
+    }
+  }),
 
   /**
    * Handle moving the focus change after a tab or return keypress in the