Bug 1241046 - find inherited entry in modifySelector2; r=pbro draft
authorTom Tromey <tom@tromey.com>
Fri, 22 Jan 2016 13:12:58 -0700
changeset 352012 2ae2ae0f431ecc2961ec71984ef4c8f07ebba185
parent 352011 0260dd297a25b97b11dfa208c3d556cb7b62542c
child 518554 1563553184ece1925483558b62ec4d0215e51984
push id15586
push userbmo:ttromey@mozilla.com
push dateFri, 15 Apr 2016 13:48:12 +0000
reviewerspbro
bugs1241046
milestone48.0a1
Bug 1241046 - find inherited entry in modifySelector2; r=pbro MozReview-Commit-ID: K0orZj2Revc
devtools/client/inspector/rules/test/browser.ini
devtools/client/inspector/rules/test/browser_rules_edit-selector_08.js
devtools/server/actors/styles.js
--- a/devtools/client/inspector/rules/test/browser.ini
+++ b/devtools/client/inspector/rules/test/browser.ini
@@ -116,16 +116,17 @@ skip-if = os == "mac" # Bug 1245996 : cl
 [browser_rules_edit-selector-commit.js]
 [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_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]
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/rules/test/browser_rules_edit-selector_08.js
@@ -0,0 +1,71 @@
+/* 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 that reverting a selector edit does the right thing.
+// Bug 1241046.
+
+const TEST_URI = `
+  <style type="text/css">
+    span {
+      color: chartreuse;
+    }
+  </style>
+  <span>
+    <div id="testid" class="testclass">Styled Node</div>
+  </span>
+`;
+
+add_task(function* () {
+  yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
+  let {inspector, view} = yield openRuleView();
+
+  info("Selecting the test element");
+  yield selectNode("#testid", inspector);
+
+  let idRuleEditor = getRuleViewRuleEditor(view, 2);
+
+  info("Focusing an existing selector name in the rule-view");
+  let editor = yield focusEditableField(view, idRuleEditor.selectorText);
+
+  is(inplaceEditor(idRuleEditor.selectorText), editor,
+    "The selector editor got focused");
+
+  info("Entering a new selector name and committing");
+  editor.input.value = "pre";
+
+  info("Waiting for rule view to update");
+  let onRuleViewChanged = once(view, "ruleview-changed");
+
+  info("Entering the commit key");
+  EventUtils.synthesizeKey("VK_RETURN", {});
+  yield onRuleViewChanged;
+
+  info("Re-focusing the selector name in the rule-view");
+  idRuleEditor = getRuleViewRuleEditor(view, 2);
+  editor = yield focusEditableField(view, idRuleEditor.selectorText);
+
+  is(view._elementStyle.rules.length, 2, "Should have 2 rules.");
+  ok(getRuleViewRule(view, "pre"), "Rule with pre selector exists.");
+  is(getRuleViewRuleEditor(view, 2).element.getAttribute("unmatched"),
+     "true",
+     "Rule with pre does not match the current element.");
+
+  // Now change it back.
+  info("Re-entering original selector name and committing");
+  editor.input.value = "span";
+
+  info("Waiting for rule view to update");
+  onRuleViewChanged = once(view, "ruleview-changed");
+
+  info("Entering the commit key");
+  EventUtils.synthesizeKey("VK_RETURN", {});
+  yield onRuleViewChanged;
+
+  is(view._elementStyle.rules.length, 2, "Should have 2 rules.");
+  ok(getRuleViewRule(view, "span"), "Rule with span selector exists.");
+  is(getRuleViewRuleEditor(view, 2).element.getAttribute("unmatched"),
+     "false", "Rule with span matches the current element.");
+});
--- a/devtools/server/actors/styles.js
+++ b/devtools/server/actors/styles.js
@@ -738,26 +738,50 @@ var PageStyleActor = protocol.ActorClass
         isSystem: isSystem,
         pseudoElement: pseudo
       });
     }
     return rules;
   },
 
   /**
+   * Given a node and a CSS rule, walk up the DOM looking for a
+   * matching element rule.  Return an array of all found entries, in
+   * the form generated by _getAllElementRules.  Note that this will
+   * always return an array of either zero or one element.
+   *
+   * @param {NodeActor} node the node
+   * @param {CSSStyleRule} filterRule the rule to filter for
+   * @return {Array} array of zero or one elements; if one, the element
+   *                 is the entry as returned by _getAllElementRules.
+   */
+  findEntryMatchingRule: function(node, filterRule) {
+    const options = {matchedSelectors: true, inherited: true};
+    let entries = [];
+    let parent = this.walker.parentNode(node);
+    while (parent && parent.rawNode.nodeType != Ci.nsIDOMNode.DOCUMENT_NODE) {
+      entries = entries.concat(this._getAllElementRules(parent, parent,
+                                                        options));
+      parent = this.walker.parentNode(parent);
+    }
+
+    return entries.filter(entry => entry.rule.rawRule === filterRule);
+  },
+
+  /**
    * Helper function for getApplied that fetches a set of style properties that
    * apply to the given node and associated rules
    * @param NodeActor node
    * @param object options
    *   `filter`: A string filter that affects the "matched" handling.
    *     'user': Include properties from user style sheets.
    *     'ua': Include properties from user and user-agent sheets.
    *     Default value is 'ua'
    *   `inherited`: Include styles inherited from parent nodes.
-   *   `matchedSeletors`: Include an array of specific selectors that
+   *   `matchedSelectors`: Include an array of specific selectors that
    *     caused this rule to match its node.
    * @param array entries
    *   List of appliedstyle objects that lists the rules that apply to the
    *   node. If adding a new rule to the stylesheet, only the new rule entry
    *   is provided and only the style properties that apply to the new
    *   rule is fetched.
    * @returns Object containing the list of rule entries, rule actors and
    *   stylesheet actors that applies to the given node and its associated
@@ -965,17 +989,17 @@ var PageStyleActor = protocol.ActorClass
 
     return this._styleElement;
   },
 
   /**
    * Helper function for adding a new rule and getting its applied style
    * properties
    * @param NodeActor node
-   * @param CSSStyleRUle rule
+   * @param CSSStyleRule rule
    * @returns Object containing its applied style properties
    */
   getNewAppliedProps: function(node, rule) {
     let ruleActor = this._styleRef(rule);
     return this.getAppliedProps(node, [{ rule: ruleActor }],
       { matchedSelectors: true });
   },
 
@@ -1621,48 +1645,48 @@ var StyleRuleActor = protocol.ActorClass
    *        authored text; false if the selector should be updated via
    *        CSSOM.
    * @returns {Object}
    *        Returns an object that contains the applied style properties of the
    *        new rule and a boolean indicating whether or not the new selector
    *        matches the current selected element
    */
   modifySelector2: method(function(node, value, editAuthored = false) {
-    let ruleProps = null;
-
     if (this.type === ELEMENT_STYLE ||
         this.rawRule.selectorText === value) {
-      return { ruleProps, isMatching: true };
+      return { ruleProps: null, isMatching: true };
     }
 
     let selectorPromise = this._addNewSelector(value, editAuthored);
 
     if (editAuthored) {
       selectorPromise = selectorPromise.then((newCssRule) => {
         if (newCssRule) {
           let style = this.pageStyle._styleRef(newCssRule);
           // See the comment in |form| to understand this.
           return style.getAuthoredCssText().then(() => newCssRule);
         }
         return newCssRule;
       });
     }
 
     return selectorPromise.then((newCssRule) => {
-      if (newCssRule) {
-        ruleProps = this.pageStyle.getNewAppliedProps(node, newCssRule);
-      }
+      let ruleProps = null;
+      let isMatching = false;
 
-      // Determine if the new selector value matches the current
-      // selected element
-      let isMatching = false;
-      try {
-        isMatching = node.rawNode.matches(value);
-      } catch (e) {
-        // This fails when value is an invalid selector.
+      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);
+        }
       }
 
       return { ruleProps, isMatching };
     });
   }, {
     request: {
       node: Arg(0, "domnode"),
       value: Arg(1, "string"),