Bug 966895 - [rule view] Adding new rules to the current selection in the CSS rule-view r=harth

browser/devtools/styleinspector/rule-view.js

@@ -442,7 +442,7 @@ Rule.prototype = {
       return this._title;
     }
     this._title = CssLogic.shortSource(this.sheet);
-    if (this.domRule.type !== ELEMENT_STYLE) {
+    if (this.domRule.type !== ELEMENT_STYLE && this.ruleLine > 0) {
       this._title += ":" + this.ruleLine;
     }
 
@@ -1076,6 +1076,7 @@ function CssRuleView(aInspector, aDoc, aStore, aPageStyle) {
 
   this._buildContextMenu = this._buildContextMenu.bind(this);
   this._contextMenuUpdate = this._contextMenuUpdate.bind(this);
+  this._onAddRule = this._onAddRule.bind(this);
   this._onSelectAll = this._onSelectAll.bind(this);
   this._onCopy = this._onCopy.bind(this);
   this._onCopyColor = this._onCopyColor.bind(this);
@@ -1125,6 +1126,11 @@ CssRuleView.prototype = {
     this._contextmenu.addEventListener("popupshowing", this._contextMenuUpdate);
     this._contextmenu.id = "rule-view-context-menu";
 
+    this.menuitemAddRule = createMenuItem(this._contextmenu, {
+      label: "ruleView.contextmenu.addRule",
+      accesskey: "ruleView.contextmenu.addRule.accessKey",
+      command: this._onAddRule
+    });
     this.menuitemSelectAll = createMenuItem(this._contextmenu, {
       label: "ruleView.contextmenu.selectAll",
       accesskey: "ruleView.contextmenu.selectAll.accessKey",
@@ -1140,7 +1146,7 @@ CssRuleView.prototype = {
       accesskey: "ruleView.contextmenu.copyColor.accessKey",
       command: this._onCopyColor
     });
-    this.menuitemSources= createMenuItem(this._contextmenu, {
+    this.menuitemSources = createMenuItem(this._contextmenu, {
       label: "ruleView.contextmenu.showOrigSources",
       accesskey: "ruleView.contextmenu.showOrigSources.accessKey",
       command: this._onToggleOrigSources
@@ -1354,6 +1360,43 @@ CssRuleView.prototype = {
     Services.prefs.setBoolPref(PREF_ORIG_SOURCES, !isEnabled);
   },
 
+  /**
+   * Add a new rule to the current element.
+   */
+  _onAddRule: function() {
+    let elementStyle = this._elementStyle;
+    let element = elementStyle.element;
+    let rules = elementStyle.rules;
+    let client = this.inspector.toolbox._target.client;
+
+    if (!client.traits.addNewRule) {
+      return;
+    }
+
+    this.pageStyle.addNewRule(element).then(options => {
+      let newRule = new Rule(elementStyle, options);
+      elementStyle.rules.push(newRule);
+      let editor = new RuleEditor(this, newRule);
+
+      // Insert the new rule editor after the inline element rule
+      if (rules.length <= 1) {
+        this.element.appendChild(editor.element);
+      } else {
+        for (let rule of rules) {
+          if (rule.selectorText === "element") {
+            let referenceElement = rule.editor.element.nextSibling;
+            this.element.insertBefore(editor.element, referenceElement);
+            break;
+          }
+        }
+      }
+
+      // Focus and make the new rule's selector editable
+      editor.selectorText.click();
+      elementStyle._changed();
+    });
+  },
+
   setPageStyle: function(aPageStyle) {
     this.pageStyle = aPageStyle;
   },
@@ -1852,6 +1895,9 @@ RuleEditor.prototype = {
       }
     } else {
       sourceLabel.setAttribute("value", this.rule.title);
+      if (this.rule.ruleLine == -1 && this.rule.domRule.parentStyleSheet) {
+        sourceLabel.parentNode.setAttribute("unselectable", "true");
+      }
     }
 
     let showOrig = Services.prefs.getBoolPref(PREF_ORIG_SOURCES);

browser/devtools/styleinspector/test/browser.ini

@@ -42,6 +42,8 @@ support-files =
 [browser_ruleview_add-property-cancel_03.js]
 [browser_ruleview_add-property_01.js]
 [browser_ruleview_add-property_02.js]
+[browser_ruleview_add-rule_01.js]
+[browser_ruleview_add-rule_02.js]
 [browser_ruleview_colorpicker-and-image-tooltip_01.js]
 [browser_ruleview_colorpicker-and-image-tooltip_02.js]
 [browser_ruleview_colorpicker-appears-on-swatch-click.js]

browser/devtools/styleinspector/test/browser_ruleview_add-rule_01.js

@@ -0,0 +1,89 @@
+/* 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 the behaviour of adding a new rule to the rule view and the
+// various inplace-editor behaviours in the new rule editor
+
+let PAGE_CONTENT = [
+  '<style type="text/css">',
+  '  .testclass {',
+  '    text-align: center;',
+  '  }',
+  '</style>',
+  '<div id="testid" class="testclass">Styled Node</div>',
+  '<span class="testclass2">This is a span</span>',
+  '<p>Empty<p>'
+].join("\n");
+
+const TEST_DATA = [
+  { node: "#testid", expected: "#testid" },
+  { node: ".testclass2", expected: ".testclass2" },
+  { node: "p", expected: "p" }
+];
+
+let test = asyncTest(function*() {
+  yield addTab("data:text/html;charset=utf-8,test rule view add rule");
+
+  info("Creating the test document");
+  content.document.body.innerHTML = PAGE_CONTENT;
+
+  info("Opening the rule-view");
+  let {toolbox, inspector, view} = yield openRuleView();
+
+  info("Iterating over the test data");
+  for (let data of TEST_DATA) {
+    yield runTestData(inspector, view, data);
+  }
+});
+
+function* runTestData(inspector, view, data) {
+  let {node, expected} = data;
+  info("Selecting the test element");
+  yield selectNode(node, inspector);
+
+  info("Waiting for context menu to be shown");
+  let onPopup = once(view._contextmenu, "popupshown");
+  let win = view.doc.defaultView;
+
+  EventUtils.synthesizeMouseAtCenter(view.element,
+    {button: 2, type: "contextmenu"}, win);
+  yield onPopup;
+
+  ok(!view.menuitemAddRule.hidden, "Add rule is visible");
+
+  info("Waiting for rule view to change");
+  let onRuleViewChanged = once(view.element, "CssRuleViewChanged");
+
+  info("Adding the new rule");
+  view.menuitemAddRule.click();
+  yield onRuleViewChanged;
+  view._contextmenu.hidePopup();
+
+  yield testNewRule(view, expected, 1);
+
+  info("Resetting page content");
+  content.document.body.innerHTML = PAGE_CONTENT;
+}
+
+function* testNewRule(view, expected, index) {
+  let idRuleEditor = getRuleViewRuleEditor(view, index);
+  let editor = idRuleEditor.selectorText.ownerDocument.activeElement;
+  is(editor.value, expected,
+      "Selector editor value is as expected: " + expected);
+
+  info("Entering the escape key");
+  EventUtils.synthesizeKey("VK_ESCAPE", {});
+
+  is(idRuleEditor.selectorText.textContent, expected,
+      "Selector text value is as expected: " + expected);
+
+  info("Adding new properties to new rule: " + expected)
+  idRuleEditor.addProperty("font-weight", "bold", "");
+  let textProps = idRuleEditor.rule.textProps;
+  let lastRule = textProps[textProps.length - 1];
+  is(lastRule.name, "font-weight", "Last rule name is font-weight");
+  is(lastRule.value, "bold", "Last rule value is bold");
+}

browser/devtools/styleinspector/test/browser_ruleview_add-rule_02.js

@@ -0,0 +1,78 @@
+/* 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 the behaviour of adding a new rule to the rule view and editing
+// its selector
+
+let PAGE_CONTENT = [
+  '<style type="text/css">',
+  '  #testid {',
+  '    text-align: center;',
+  '  }',
+  '</style>',
+  '<div id="testid">Styled Node</div>',
+  '<span>This is a span</span>'
+].join("\n");
+
+let test = asyncTest(function*() {
+  yield addTab("data:text/html;charset=utf-8,test rule view add rule");
+
+  info("Creating the test document");
+  content.document.body.innerHTML = PAGE_CONTENT;
+
+  info("Opening the rule-view");
+  let {toolbox, inspector, view} = yield openRuleView();
+
+  info("Selecting the test element");
+  yield selectNode("#testid", inspector);
+
+  info("Waiting for context menu to be shown");
+  let onPopup = once(view._contextmenu, "popupshown");
+  let win = view.doc.defaultView;
+
+  EventUtils.synthesizeMouseAtCenter(view.element,
+    {button: 2, type: "contextmenu"}, win);
+  yield onPopup;
+
+  ok(!view.menuitemAddRule.hidden, "Add rule is visible");
+
+  info("Waiting for rule view to change");
+  let onRuleViewChanged = once(view.element, "CssRuleViewChanged");
+
+  info("Adding the new rule");
+  view.menuitemAddRule.click();
+  yield onRuleViewChanged;
+  view._contextmenu.hidePopup();
+
+  yield testEditSelector(view, "span");
+
+  info("Selecting the modified element");
+  yield selectNode("span", inspector);
+  yield checkModifiedElement(view, "span");
+});
+
+function* testEditSelector(view, name) {
+  info("Test editing existing selector field");
+  let idRuleEditor = getRuleViewRuleEditor(view, 1);
+  let editor = idRuleEditor.selectorText.ownerDocument.activeElement;
+
+  info("Entering a new selector name and committing");
+  editor.value = name;
+
+  info("Waiting for rule view to refresh");
+  let onRuleViewRefresh = once(view.element, "CssRuleViewRefreshed");
+
+  info("Entering the commit key");
+  EventUtils.synthesizeKey("VK_RETURN", {});
+  yield onRuleViewRefresh;
+
+  is(view._elementStyle.rules.length, 2, "Should have 2 rules.");
+}
+
+function* checkModifiedElement(view, name) {
+  is(view._elementStyle.rules.length, 2, "Should have 2 rules.");
+  ok(getRuleViewRule(view, name), "Rule with " + name + " selector exists.");
+}

toolkit/devtools/server/actors/root.js

@@ -124,7 +124,10 @@ RootActor.prototype = {
     bulk: true,
     // Whether the style rule actor implements the modifySelector method
     // that modifies the rule's selector
-    selectorEditable: true
+    selectorEditable: true,
+    // Whether the page style actor implements the addNewRule method that
+    // adds new rules to the page
+    addNewRule: true
   },
 
   /**

toolkit/devtools/server/actors/styles.js

@@ -29,6 +29,9 @@ exports.PSEUDO_ELEMENTS = PSEUDO_ELEMENTS;
 // Predeclare the domnode actor type for use in requests.
 types.addActorType("domnode");
 
+// Predeclare the domstylerule actor type
+types.addActorType("domstylerule");
+
 /**
  * DOM Nodes returned by the style actor will be owned by the DOM walker
  * for the connection.
@@ -52,6 +55,12 @@ types.addDictType("matchedselector", {
   status: "number"
 });
 
+types.addDictType("appliedStylesReturn", {
+  entries: "array:appliedstyle",
+  rules: "array:domstylerule",
+  sheets: "array:stylesheet"
+});
+
 /**
  * The PageStyle actor lets the client look at the styles on a page, as
  * they are applied to a given node.
@@ -79,6 +88,9 @@ var PageStyleActor = protocol.ActorClass({
 
     // Stores the association of DOM objects -> actors
     this.refMap = new Map;
+
+    this.onFrameUnload = this.onFrameUnload.bind(this);
+    events.on(this.inspector.tabActor, "will-navigate", this.onFrameUnload);
   },
 
   get conn() this.inspector.conn,
@@ -279,7 +291,6 @@ var PageStyleActor = protocol.ActorClass({
   /**
    * Get the set of styles that apply to a given node.
    * @param NodeActor node
-   * @param string property
    * @param object options
    *   `filter`: A string filter that affects the "matched" handling.
    *     'user': Include properties from user style sheets.
@@ -291,46 +302,8 @@ var PageStyleActor = protocol.ActorClass({
    */
   getApplied: method(function(node, options) {
     let entries = [];
-
     this.addElementRules(node.rawNode, undefined, options, entries);
-
-    if (options.inherited) {
-      let parent = this.walker.parentNode(node);
-      while (parent && parent.rawNode.nodeType != Ci.nsIDOMNode.DOCUMENT_NODE) {
-        this.addElementRules(parent.rawNode, parent, options, entries);
-        parent = this.walker.parentNode(parent);
-      }
-    }
-
-    if (options.matchedSelectors) {
-      for (let entry of entries) {
-        if (entry.rule.type === ELEMENT_STYLE) {
-          continue;
-        }
-
-        let domRule = entry.rule.rawRule;
-        let selectors = CssLogic.getSelectors(domRule);
-        let element = entry.inherited ? entry.inherited.rawNode : node.rawNode;
-        entry.matchedSelectors = [];
-        for (let i = 0; i < selectors.length; i++) {
-          if (DOMUtils.selectorMatchesElement(element, domRule, i)) {
-            entry.matchedSelectors.push(selectors[i]);
-          }
-        }
-
-      }
-    }
-
-    let rules = new Set;
-    let sheets = new Set;
-    entries.forEach(entry => rules.add(entry.rule));
-    this.expandSets(rules, sheets);
-
-    return {
-      entries: entries,
-      rules: [...rules],
-      sheets: [...sheets]
-    }
+    return this.getAppliedProps(node, entries, options);
   }, {
     request: {
       node: Arg(0, "domnode"),
@@ -338,11 +311,7 @@ var PageStyleActor = protocol.ActorClass({
       matchedSelectors: Option(1, "boolean"),
       filter: Option(1, "string")
     },
-    response: RetVal(types.addDictType("appliedStylesReturn", {
-      entries: "array:appliedstyle",
-      rules: "array:domstylerule",
-      sheets: "array:stylesheet"
-    }))
+    response: RetVal("appliedStylesReturn")
   }),
 
   _hasInheritedProps: function(style) {
@@ -414,6 +383,66 @@ var PageStyleActor = protocol.ActorClass({
     }
   },
 
+  /**
+   * Helper function for getApplied and addNewRule 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
+   *     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
+   *   rules.
+   */
+  getAppliedProps: function(node, entries, options) {
+    if (options.inherited) {
+      let parent = this.walker.parentNode(node);
+      while (parent && parent.rawNode.nodeType != Ci.nsIDOMNode.DOCUMENT_NODE) {
+        this.addElementRules(parent.rawNode, parent, options, entries);
+        parent = this.walker.parentNode(parent);
+      }
+    }
+
+    if (options.matchedSelectors) {
+      for (let entry of entries) {
+        if (entry.rule.type === ELEMENT_STYLE) {
+          continue;
+        }
+
+        let domRule = entry.rule.rawRule;
+        let selectors = CssLogic.getSelectors(domRule);
+        let element = entry.inherited ? entry.inherited.rawNode : node.rawNode;
+        entry.matchedSelectors = [];
+        for (let i = 0; i < selectors.length; i++) {
+          if (DOMUtils.selectorMatchesElement(element, domRule, i)) {
+            entry.matchedSelectors.push(selectors[i]);
+          }
+        }
+      }
+    }
+
+    let rules = new Set;
+    let sheets = new Set;
+    entries.forEach(entry => rules.add(entry.rule));
+    this.expandSets(rules, sheets);
+
+    return {
+      entries: entries,
+      rules: [...rules],
+      sheets: [...sheets]
+    }
+  },
+
   /**
    * Expand Sets of rules and sheets to include all parent rules and sheets.
    */
@@ -516,6 +545,59 @@ var PageStyleActor = protocol.ActorClass({
     return margins;
   },
 
+  /**
+   * On page navigation, tidy up remaining objects.
+   */
+  onFrameUnload: function() {
+    this._styleElement = null;
+  },
+
+  /**
+   * Helper function to addNewRule to construct a new style tag in the document.
+   * @returns DOMElement of the style tag
+   */
+  get styleElement() {
+    if (!this._styleElement) {
+      let document = this.inspector.window.document;
+      let style = document.createElement("style");
+      style.setAttribute("type", "text/css");
+      document.head.appendChild(style);
+      this._styleElement = style;
+    }
+
+    return this._styleElement;
+  },
+
+  /**
+   * 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;
+
+    let selector;
+    if (rawNode.id) {
+      selector = "#" + rawNode.id;
+    } else if (rawNode.className) {
+      selector = "." + rawNode.className;
+    } 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 });
+  }, {
+    request: {
+      node: Arg(0, "domnode")
+    },
+    response: RetVal("appliedStylesReturn")
+  }),
+
 });
 exports.PageStyleActor = PageStyleActor;
 
@@ -550,12 +632,17 @@ var PageStyleFront = protocol.FrontClass(PageStyleActor, {
     });
   }, {
     impl: "_getApplied"
+  }),
+
+  addNewRule: protocol.custom(function(node) {
+    return this._addNewRule(node).then(ret => {
+      return ret.entries[0];
+    });
+  }, {
+    impl: "_addNewRule"
   })
 });
 
-// Predeclare the domstylerule actor type
-types.addActorType("domstylerule");
-
 /**
  * An actor that represents a CSS style object on the protocol.
  *

toolkit/locales/en-US/chrome/global/devtools/styleinspector.properties

@@ -104,6 +104,14 @@ ruleView.contextmenu.showCSSSources=Show CSS sources
 # the rule view context menu "Show CSS sources" entry.
 ruleView.contextmenu.showCSSSources.accessKey=S
 
+# LOCALIZATION NOTE (ruleView.contextmenu.addRule): Text displayed in the
+# rule view context menu for adding a new rule to the element.
+ruleView.contextmenu.addRule=Add rule
+
+# LOCALIZATION NOTE (ruleView.contextmenu.addRule.accessKey): Access key for
+# the rule view context menu "Add rule" entry.
+ruleView.contextmenu.addRule.accessKey=R
+
 # LOCALIZATION NOTE (computedView.contextmenu.selectAll): Text displayed in the
 # computed view context menu.
 computedView.contextmenu.selectAll=Select all