Bug 1241046 - Find inherited entry in modifySelector2. r=pbro
authorTom Tromey <tom@tromey.com>
Fri, 22 Jan 2016 13:12:58 -0700
changeset 331338 6a623b9d45990722da19fc864c7ef820821bd4b0
parent 331337 7cee6f0b47093dfa2bf247e475c38508adfb0811
child 331339 b2c38c677cc7ba0b2b5ceb8f84cdace57ecd8121
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspbro
bugs1241046
milestone48.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 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"),