Bug 1139058 - Part 1: Allow unmatched rules to be added to the rule view r=pbrosset
authorGabriel Luong <gabriel.luong@gmail.com>
Wed, 27 May 2015 17:36:17 -0700
changeset 246010 e56dd3f52961c9a4231f5a04bc25e2387ab6bbf0
parent 246009 242916838c5c24e1bc79cea0932fc7a00bf393d7
child 246011 f30178f41b1f291e79b13adc23d0029f4dc0fdf4
push id60333
push userryanvm@gmail.com
push dateThu, 28 May 2015 14:20:47 +0000
treeherdermozilla-inbound@8225a3b75df6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspbrosset
bugs1139058
milestone41.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 1139058 - Part 1: Allow unmatched rules to be added to the rule view r=pbrosset
browser/devtools/styleinspector/rule-view.js
browser/themes/shared/devtools/ruleview.css
toolkit/devtools/server/actors/styles.js
--- a/browser/devtools/styleinspector/rule-view.js
+++ b/browser/devtools/styleinspector/rule-view.js
@@ -2680,23 +2680,55 @@ RuleEditor.prototype = {
    *        True if the change should be applied.
    */
   _onSelectorDone: function(aValue, aCommit) {
     if (!aCommit || this.isEditing || aValue === "" ||
         aValue === this.rule.selectorText) {
       return;
     }
 
+    let ruleView = this.ruleView;
+    let elementStyle = ruleView._elementStyle;
+    let element = elementStyle.element;
+    let supportsUnmatchedRules =
+      this.rule.domRule.supportsModifySelectorUnmatched;
+
     this.isEditing = true;
 
-    this.rule.domRule.modifySelector(aValue).then(isModified => {
+    this.rule.domRule.modifySelector(element, aValue).then(response => {
       this.isEditing = false;
 
-      if (isModified) {
-        this.ruleView.refreshPanel();
+      if (!supportsUnmatchedRules) {
+        if (response) {
+          this.ruleView.refreshPanel();
+        }
+        return;
+      }
+
+      let {ruleProps, isMatching} = response;
+      if (!ruleProps) {
+        return;
+      }
+
+      let newRule = new Rule(elementStyle, ruleProps);
+      let editor = new RuleEditor(ruleView, newRule);
+      let rules = elementStyle.rules;
+
+      rules.splice(rules.indexOf(this.rule), 1);
+      rules.push(newRule);
+      elementStyle._changed();
+
+      editor.element.setAttribute("unmatched", !isMatching);
+      this.element.parentNode.replaceChild(editor.element, this.element);
+
+      // Remove highlight for modified selector
+      if (ruleView.highlightedSelector &&
+          ruleView.highlightedSelector == this.rule.selectorText) {
+        ruleView.toggleSelectorHighlighter(ruleView.lastSelectorIcon,
+          ruleView.highlightedSelector);
       }
     }).then(null, err => {
       this.isEditing = false;
       promiseWarn(err);
     });
   }
 };
 
--- a/browser/themes/shared/devtools/ruleview.css
+++ b/browser/themes/shared/devtools/ruleview.css
@@ -54,17 +54,21 @@
   border-left: solid 10px;
 }
 
 .ruleview-rule,
 #noResults {
   padding: 2px 4px;
 }
 
-/* User agent styles are not editable, display them differently */
+/**
+ * Display rules that don't match the current selected element and uneditable
+ * user agent styles differently
+ */
+.ruleview-rule[unmatched=true],
 .ruleview-rule[uneditable=true] {
   background: var(--theme-tab-toolbar-background);
 }
 
 .ruleview-rule[uneditable=true] :focus {
   outline: none;
 }
 
--- a/toolkit/devtools/server/actors/styles.js
+++ b/toolkit/devtools/server/actors/styles.js
@@ -54,16 +54,20 @@ types.addActorType("domstylerule");
  * for the connection.
   */
 types.addLifetime("walker", "walker");
 
 /**
  * When asking for the styles applied to a node, we return a list of
  * appliedstyle json objects that lists the rules that apply to the node
  * and which element they were inherited from (if any).
+ *
+ * Note appliedstyle only sends the list of actorIDs and is not a valid return
+ * value on its own. appliedstyle should be returned with the actual list of
+ * StyleRuleActor and StyleSheetActor. See appliedStylesReturn.
  */
 types.addDictType("appliedstyle", {
   rule: "domstylerule#actorid",
   inherited: "nullable:domnode#actorid",
   keyframes: "nullable:domstylerule#actorid"
 });
 
 types.addDictType("matchedselector", {
@@ -74,16 +78,21 @@ types.addDictType("matchedselector", {
 });
 
 types.addDictType("appliedStylesReturn", {
   entries: "array:appliedstyle",
   rules: "array:domstylerule",
   sheets: "array:stylesheet"
 });
 
+types.addDictType("modifiedStylesReturn", {
+  isMatching: RetVal("boolean"),
+  ruleProps: RetVal("nullable:appliedStylesReturn")
+});
+
 types.addDictType("fontpreview", {
   data: "nullable:longstring",
   size: "json"
 });
 
 types.addDictType("fontface", {
   name: "string",
   CSSFamilyName: "string",
@@ -622,20 +631,19 @@ var PageStyleActor = protocol.ActorClass
         inherited: inherited,
         isSystem: isSystem,
         pseudoElement: pseudo
       });
     }
     return rules;
   },
 
-
   /**
-   * Helper function for getApplied and addNewRule that fetches a set of
-   * style properties that apply to the given node and associated rules
+   * 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
@@ -833,16 +841,29 @@ var PageStyleActor = protocol.ActorClass
       document.documentElement.appendChild(style);
       this._styleElement = style;
     }
 
     return this._styleElement;
   },
 
   /**
+   * Helper function for adding a new rule and getting its applied style
+   * properties
+   * @param NodeActor node
+   * @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 });
+  },
+
+  /**
    * Adds a new rule, and returns the new StyleRuleActor.
    * @param   NodeActor node
    * @returns StyleRuleActor of the new rule
    */
   addNewRule: method(function(node) {
     let style = this.styleElement;
     let sheet = style.sheet;
     let rawNode = node.rawNode;
@@ -852,26 +873,23 @@ var PageStyleActor = protocol.ActorClass
       selector = "#" + rawNode.id;
     } else if (rawNode.className) {
       selector = "." + rawNode.className.split(" ").join(".");
     } else {
       selector = rawNode.tagName.toLowerCase();
     }
 
     let index = sheet.insertRule(selector + " {}", sheet.cssRules.length);
-    let ruleActor = this._styleRef(sheet.cssRules[index]);
-    return this.getAppliedProps(node, [{ rule: ruleActor }],
-      { matchedSelectors: true });
+    return this.getNewAppliedProps(node, sheet.cssRules[index]);
   }, {
     request: {
       node: Arg(0, "domnode")
     },
     response: RetVal("appliedStylesReturn")
   }),
-
 });
 exports.PageStyleActor = PageStyleActor;
 
 /**
  * Front object for the PageStyleActor
  */
 var PageStyleFront = protocol.FrontClass(PageStyleActor, {
   initialize: function(conn, form, ctx, detail) {
@@ -985,17 +1003,22 @@ var StyleRuleActor = protocol.ActorClass
     if (detail === "actorid") {
       return this.actorID;
     }
 
     let form = {
       actor: this.actorID,
       type: this.type,
       line: this.line || undefined,
-      column: this.column
+      column: this.column,
+      traits: {
+        // Whether the style rule actor implements the modifySelector2 method
+        // that allows for unmatched rule to be added
+        modifySelectorUnmatched: true,
+      }
     };
 
     if (this.rawRule.parentRule) {
       form.parentRule = this.pageStyle._styleRef(this.rawRule.parentRule).actorID;
 
       // CSS rules that we call media rules are STYLE_RULES that are children
       // of MEDIA_RULEs. We need to check the parentRule to check if a rule is
       // a media rule so we do this here instead of in the switch statement
@@ -1092,72 +1115,140 @@ var StyleRuleActor = protocol.ActorClass
 
     return this;
   }, {
     request: { modifications: Arg(0, "array:json") },
     response: { rule: RetVal("domstylerule") }
   }),
 
   /**
-   * Removes the current rule and inserts a new rule with the new selector
-   * into the parent style sheet.
+   * Helper function for modifySelector and modifySelector2, inserts the new
+   * rule with the new selector into the parent style sheet and removes the
+   * current rule. Returns the newly inserted css rule or null if the rule is
+   * unsuccessfully inserted to the parent style sheet.
+   *
+   * @param string value
+   *        The new selector value
+   *
+   * @returns CSSRule
+   *        The new CSS rule added
+   */
+  _addNewSelector: function(value) {
+    let rule = this.rawRule;
+    let parentStyleSheet = rule.parentStyleSheet;
+    let cssRules = parentStyleSheet.cssRules;
+    let cssText = rule.cssText;
+    let selectorText = rule.selectorText;
+
+    for (let i = 0; i < cssRules.length; i++) {
+      if (rule === cssRules.item(i)) {
+        try {
+          // Inserts the new style rule into the current style sheet and
+          // delete the current rule
+          let ruleText = cssText.slice(selectorText.length).trim();
+          parentStyleSheet.insertRule(value + " " + ruleText, i);
+          parentStyleSheet.deleteRule(i + 1);
+          return cssRules.item(i);
+        } catch(e) {}
+
+        break;
+      }
+    }
+
+    return null;
+  },
+
+  /**
+   * Modify the current rule's selector by inserting a new rule with the new
+   * selector value and removing the current rule.
+   *
+   * Note this method was kept for backward compatibility, but unmatched rules
+   * support was added in FF41.
+   *
    * @param string value
    *        The new selector value
    * @returns boolean
    *        Returns a boolean if the selector in the stylesheet was modified,
    *        and false otherwise
    */
   modifySelector: method(function(value) {
     if (this.type === ELEMENT_STYLE) {
       return false;
     }
 
-    let rule = this.rawRule;
-    let parentStyleSheet = rule.parentStyleSheet;
-    let document = this.getDocument(parentStyleSheet);
+    let document = this.getDocument(this.rawRule.parentStyleSheet);
     // Extract the selector, and pseudo elements and classes
     let [selector, pseudoProp] = value.split(/(:{1,2}.+$)/);
     let selectorElement;
 
     try {
       selectorElement = document.querySelector(selector);
     } catch (e) {
       return false;
     }
 
     // Check if the selector is valid and not the same as the original
     // selector
-    if (selectorElement && rule.selectorText !== value) {
-      let cssRules = parentStyleSheet.cssRules;
-      let cssText = rule.cssText;
-      let selectorText = rule.selectorText;
-
-      // Delete the currently selected rule
-      let i = 0;
-      for (let cssRule of cssRules) {
-        if (rule === cssRule) {
-          parentStyleSheet.deleteRule(i);
-          break;
-        }
-
-        i++;
-      }
-
-      // Inserts the new style rule into the current style sheet
-      let ruleText = cssText.slice(selectorText.length).trim();
-      parentStyleSheet.insertRule(value + " " + ruleText, i);
-
+    if (selectorElement && this.rawRule.selectorText !== value) {
+      this._addNewSelector(value);
       return true;
     } else {
       return false;
     }
   }, {
     request: { selector: Arg(0, "string") },
     response: { isModified: RetVal("boolean") },
   }),
+
+  /**
+   * Modify the current rule's selector by inserting a new rule with the new
+   * selector value and removing the current rule.
+   *
+   * In contrast with the modifySelector method which was used before FF41,
+   * this method also returns information about the new rule and applied style
+   * so that consumers can immediately display the new rule, whether or not the
+   * selector matches the current element without having to refresh the whole
+   * list.
+   *
+   * @param DOMNode node
+   *        The current selected element
+   * @param string value
+   *        The new selector value
+   * @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) {
+    let isMatching = false;
+    let ruleProps = null;
+
+    if (this.type === ELEMENT_STYLE ||
+        this.rawRule.selectorText === value) {
+      return { ruleProps, isMatching: true };
+    }
+
+    let newCssRule = this._addNewSelector(value);
+    if (newCssRule) {
+      ruleProps = this.pageStyle.getNewAppliedProps(node, newCssRule);
+    }
+
+    // Determine if the new selector value matches the current selected element
+    try {
+      isMatching = node.rawNode.matches(value);
+    } catch(e) {}
+
+    return { ruleProps, isMatching };
+  }, {
+    request: {
+      node: Arg(0, "domnode"),
+      value: Arg(1, "string")
+    },
+    response: RetVal("modifiedStylesReturn")
+  })
 });
 
 /**
  * Front for the StyleRule actor.
  */
 var StyleRuleFront = protocol.FrontClass(StyleRuleActor, {
   initialize: function(client, form, ctx, detail) {
     protocol.Front.prototype.initialize.call(this, client, form, ctx, detail);
@@ -1235,16 +1326,20 @@ var StyleRuleFront = protocol.FrontClass
     return sheet ? sheet.href : "";
   },
 
   get nodeHref() {
     let sheet = this.parentStyleSheet;
     return sheet ? sheet.nodeHref : "";
   },
 
+  get supportsModifySelectorUnmatched() {
+    return this._form.traits && this._form.traits.modifySelectorUnmatched;
+  },
+
   get location()
   {
     return {
       source: this.parentStyleSheet,
       href: this.href,
       line: this.line,
       column: this.column
     };
@@ -1273,17 +1368,34 @@ var StyleRuleFront = protocol.FrontClass
           location.source = this.parentStyleSheet;
         }
         if (!source) {
           location.href = this.href;
         }
         this._originalLocation = location;
         return location;
       });
-  }
+  },
+
+  modifySelector: protocol.custom(Task.async(function*(node, value) {
+    let response;
+    if (this.supportsModifySelectorUnmatched) {
+      // If the debugee supports adding unmatched rules (post FF41)
+      response = yield this.modifySelector2(node, value);
+    } else {
+      response = yield this._modifySelector(value);
+    }
+
+    if (response.ruleProps) {
+      response.ruleProps = response.ruleProps.entries[0];
+    }
+    return response;
+  }), {
+    impl: "_modifySelector"
+  })
 });
 
 /**
  * Convenience API for building a list of attribute modifications
  * for the `modifyAttributes` request.
  */
 var RuleModificationList = Class({
   initialize: function(rule) {