Bug 1136101 - Add an "Add rule" button in the rules view toolbar. r=bgrins, ui-r=shorlander
authorTim Nguyen <ntim.bugs@gmail.com>
Sat, 16 May 2015 02:10:00 -0400
changeset 244224 db2bac6070b9
parent 244223 6973f1341ace
child 244225 4a382e5000bc
push id12993
push userryanvm@gmail.com
push dateSun, 17 May 2015 22:37:35 +0000
treeherderfx-team@4f32ea4fc71d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbgrins, shorlander
bugs1136101
milestone41.0a1
Bug 1136101 - Add an "Add rule" button in the rules view toolbar. r=bgrins, ui-r=shorlander
browser/devtools/shared/widgets/filter-widget.css
browser/devtools/styleinspector/cssruleview.xhtml
browser/devtools/styleinspector/rule-view.js
browser/devtools/styleinspector/test/browser_ruleview_add-rule_01.js
browser/devtools/styleinspector/test/browser_ruleview_add-rule_02.js
browser/devtools/styleinspector/test/browser_ruleview_add-rule_03.js
browser/locales/en-US/chrome/browser/devtools/styleinspector.dtd
browser/themes/shared/devtools/images/add.svg
browser/themes/shared/devtools/ruleview.css
toolkit/locales/en-US/chrome/global/devtools/styleinspector.properties
--- a/browser/devtools/shared/widgets/filter-widget.css
+++ b/browser/devtools/shared/widgets/filter-widget.css
@@ -107,16 +107,16 @@
   box-sizing: border-box;
   font: inherit;
   margin: 0 3px;
 }
 
 #add-filter {
   -moz-appearance: none;
   background: url(chrome://browser/skin/devtools/add.svg);
-  background-size: 18px;
+  background-size: cover;
   border: none;
   width: 16px;
   height: 16px;
   font-size: 0;
   vertical-align: middle;
   cursor: pointer;
 }
--- a/browser/devtools/styleinspector/cssruleview.xhtml
+++ b/browser/devtools/styleinspector/cssruleview.xhtml
@@ -40,16 +40,18 @@
     <div id="root" class="devtools-monospace">
       <div class="devtools-toolbar">
         <div class="devtools-searchbox">
           <input id="ruleview-searchbox"
                  class="devtools-searchinput devtools-rule-searchbox"
                  type="search" placeholder="&filterStylesPlaceholder;"/>
           <button id="ruleview-searchinput-clear" class="devtools-searchinput-clear"></button>
         </div>
+        <!-- TODO : Bug 1165122 : Show this button by default -->
+        <button hidden="true" id="ruleview-add-rule-button" title="&addRuleButtonTooltip;" class="devtools-button"></button>
       </div>
     </div>
 
     <div id="ruleview-container" class="ruleview devtools-monospace">
     </div>
 
   </body>
 </html>
--- a/browser/devtools/styleinspector/rule-view.js
+++ b/browser/devtools/styleinspector/rule-view.js
@@ -1119,23 +1119,25 @@ function CssRuleView(aInspector, aDoc, a
   this._onToggleOrigSources = this._onToggleOrigSources.bind(this);
   this._onShowMdnDocs = this._onShowMdnDocs.bind(this);
   this._onFilterStyles = this._onFilterStyles.bind(this);
   this._onFilterKeyPress = this._onFilterKeyPress.bind(this);
   this._onClearSearch = this._onClearSearch.bind(this);
   this._onFilterTextboxContextMenu = this._onFilterTextboxContextMenu.bind(this);
 
   this.element = this.doc.getElementById("ruleview-container");
+  this.addRuleButton = this.doc.getElementById("ruleview-add-rule-button");
   this.searchField = this.doc.getElementById("ruleview-searchbox");
   this.searchClearButton = this.doc.getElementById("ruleview-searchinput-clear");
 
   this.searchClearButton.hidden = true;
 
   this.element.addEventListener("copy", this._onCopy);
   this.element.addEventListener("contextmenu", this._onContextMenu);
+  this.addRuleButton.addEventListener("click", this._onAddRule);
   this.searchField.addEventListener("input", this._onFilterStyles);
   this.searchField.addEventListener("keypress", this._onFilterKeyPress);
   this.searchField.addEventListener("contextmenu", this._onFilterTextboxContextMenu);
   this.searchClearButton.addEventListener("click", this._onClearSearch);
 
   this._handlePrefChange = this._handlePrefChange.bind(this);
   this._onSourcePrefChanged = this._onSourcePrefChanged.bind(this);
 
@@ -1181,18 +1183,18 @@ CssRuleView.prototype = {
   _buildContextMenu: function() {
     let doc = this.doc.defaultView.parent.document;
 
     this._contextmenu = doc.createElementNS(XUL_NS, "menupopup");
     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",
+      label: "ruleView.contextmenu.addNewRule",
+      accesskey: "ruleView.contextmenu.addNewRule.accessKey",
       command: this._onAddRule
     });
     this.menuitemSelectAll = createMenuItem(this._contextmenu, {
       label: "ruleView.contextmenu.selectAll",
       accesskey: "ruleView.contextmenu.selectAll.accessKey",
       command: this._onSelectAll
     });
     this.menuitemCopy = createMenuItem(this._contextmenu, {
@@ -1733,16 +1735,17 @@ CssRuleView.prototype = {
     this.doc.popupNode = null;
 
     this.tooltips.destroy();
     this.highlighters.destroy();
 
     // Remove bound listeners
     this.element.removeEventListener("copy", this._onCopy);
     this.element.removeEventListener("contextmenu", this._onContextMenu);
+    this.addRuleButton.removeEventListener("click", this._onAddRule);
     this.searchField.removeEventListener("input", this._onFilterStyles);
     this.searchField.removeEventListener("keypress", this._onFilterKeyPress);
     this.searchField.removeEventListener("contextmenu",
       this._onFilterTextboxContextMenu);
     this.searchClearButton.removeEventListener("click", this._onClearSearch);
     this.searchField = null;
     this.searchClearButton = null;
 
--- a/browser/devtools/styleinspector/test/browser_ruleview_add-rule_01.js
+++ b/browser/devtools/styleinspector/test/browser_ruleview_add-rule_01.js
@@ -22,59 +22,66 @@ let PAGE_CONTENT = [
 const TEST_DATA = [
   { node: "#testid", expected: "#testid" },
   { node: ".testclass2", expected: ".testclass2" },
   { node: ".class1.class2", expected: ".class1" },
   { node: "p", expected: "p" }
 ];
 
 add_task(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;
+  yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(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);
+    yield runTestData(inspector, view, data, "context-menu");
+    yield runTestData(inspector, view, data, "button");
   }
 });
 
-function* runTestData(inspector, view, data) {
+function* runTestData(inspector, view, data, method) {
   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, "ruleview-changed");
-
-  info("Adding the new rule");
-  view.menuitemAddRule.click();
-  yield onRuleViewChanged;
-  view._contextmenu.hidePopup();
+  yield addNewRule(inspector, view, method);
 
   yield testNewRule(view, expected, 1);
 
   info("Resetting page content");
   content.document.body.innerHTML = PAGE_CONTENT;
 }
 
+function* addNewRule(inspector, view, method) {
+  if (method == "context-menu") {
+    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("Adding the new rule");
+    view.menuitemAddRule.click();
+    view._contextmenu.hidePopup();
+  }
+  else {
+    info("Adding the new rule using the button");
+    view.addRuleButton.click();
+  }
+  info("Waiting for rule view to change");
+  let onRuleViewChanged = once(view, "ruleview-changed");
+  yield onRuleViewChanged;
+}
+
 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", {});
@@ -83,9 +90,9 @@ function* testNewRule(view, expected, in
       "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");
-}
+}
\ No newline at end of file
--- a/browser/devtools/styleinspector/test/browser_ruleview_add-rule_02.js
+++ b/browser/devtools/styleinspector/test/browser_ruleview_add-rule_02.js
@@ -13,44 +13,31 @@ let PAGE_CONTENT = [
   '    text-align: center;',
   '  }',
   '</style>',
   '<div id="testid">Styled Node</div>',
   '<span>This is a span</span>'
 ].join("\n");
 
 add_task(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;
+  yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(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, "ruleview-changed");
 
   info("Adding the new rule");
-  view.menuitemAddRule.click();
+  view.addRuleButton.click();
+
   yield onRuleViewChanged;
-  view._contextmenu.hidePopup();
 
   yield testEditSelector(view, "span");
 
   info("Selecting the modified element");
   yield selectNode("span", inspector);
   yield checkModifiedElement(view, "span");
 });
 
--- a/browser/devtools/styleinspector/test/browser_ruleview_add-rule_03.js
+++ b/browser/devtools/styleinspector/test/browser_ruleview_add-rule_03.js
@@ -13,44 +13,31 @@ let PAGE_CONTENT = [
   '    text-align: center;',
   '  }',
   '</style>',
   '<div id="testid">Styled Node</div>',
   '<span>This is a span</span>'
 ].join("\n");
 
 add_task(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;
+  yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(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, "ruleview-changed");
 
   info("Adding the new rule");
-  view.menuitemAddRule.click();
+  view.addRuleButton.click();
+
   yield onRuleViewChanged;
-  view._contextmenu.hidePopup();
 
   info("Adding new properties to the new rule");
   yield testNewRule(view, "#testid", 1);
 
   info("Editing existing selector field");
   yield testEditSelector(view, "span");
 
   info("Selecting the modified element");
--- a/browser/locales/en-US/chrome/browser/devtools/styleinspector.dtd
+++ b/browser/locales/en-US/chrome/browser/devtools/styleinspector.dtd
@@ -12,16 +12,21 @@
   -  that specifies whether the styles that are not from the user's stylesheet
   -  should be displayed or not. -->
 <!ENTITY browserStylesLabel    "Browser styles">
 
 <!-- LOCALIZATION NOTE (filterStylesPlaceholder): This is the placeholder that goes in
   -  the search box when no search term has been entered. -->
 <!ENTITY filterStylesPlaceholder      "Filter Styles">
 
+<!-- LOCALIZATION NOTE (addRuleButtonTooltip): This is the tooltip shown when
+  -  hovering the `Add new rule` button in the rules view toolbar. This should
+  -  match ruleView.contextmenu.addNewRule in styleinspector.properties -->
+<!ENTITY addRuleButtonTooltip  "Add new rule">
+
 <!-- LOCALIZATION NOTE (selectedElementLabel): This is the label for the path of
   -  the highlighted element in the web page. This path is based on the document
   -  tree. -->
 <!ENTITY selectedElementLabel  "Selected element:">
 
 <!-- LOCALIZATION NOTE (noPropertiesFound): In the case where there are no CSS
   -  properties to display e.g. due to search criteria this message is
   -  displayed. -->
--- a/browser/themes/shared/devtools/images/add.svg
+++ b/browser/themes/shared/devtools/images/add.svg
@@ -1,3 +1,9 @@
-<svg width="18" height="18" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
-  <polygon fill="#EEF0F2" points="4,7 8,7 8,3 10,3 10,7 14,7 14,9 10,9 10,13 8,13 8,9 4,9 4,7"></polygon>
-</svg>
+<!-- This Source Code Form is subject to the terms of the Mozilla Public
+   - License, v. 2.0. If a copy of the MPL was not distributed with this
+   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
+<svg width="16" height="16" viewBox="0 0 16 16" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+  <g fill="#edf0f1">
+    <rect x="2" y="7" width="12" height="2" />
+    <rect x="7" y="2" width="2" height="12" />
+  </g>
+</svg>
\ No newline at end of file
--- a/browser/themes/shared/devtools/ruleview.css
+++ b/browser/themes/shared/devtools/ruleview.css
@@ -259,8 +259,13 @@
 .ruleview-selectorhighlighter:hover {
   background-position: -32px 0;
 }
 
 .ruleview-selectorhighlighter:active,
 .ruleview-selectorhighlighter.highlighted {
   background-position: -16px 0;
 }
+
+#ruleview-add-rule-button::before {
+  background-image: url("chrome://browser/skin/devtools/add.svg");
+  background-size: cover;
+}
\ No newline at end of file
--- a/toolkit/locales/en-US/chrome/global/devtools/styleinspector.properties
+++ b/toolkit/locales/en-US/chrome/global/devtools/styleinspector.properties
@@ -107,23 +107,24 @@ ruleView.contextmenu.showOrigSources.acc
 # LOCALIZATION NOTE (ruleView.contextmenu.showMdnDocs): Text displayed in the rule view
 # context menu to display docs from MDN for an item.
 ruleView.contextmenu.showMdnDocs=Show MDN docs
 
 # LOCALIZATION NOTE (ruleView.contextmenu.showMdnDocs.accessKey): Access key for
 # the rule view context menu "Show MDN docs" entry.
 ruleView.contextmenu.showMdnDocs.accessKey=D
 
-# LOCALIZATION NOTE (ruleView.contextmenu.addRule): Text displayed in the
+# LOCALIZATION NOTE (ruleView.contextmenu.addNewRule): Text displayed in the
 # rule view context menu for adding a new rule to the element.
-ruleView.contextmenu.addRule=Add rule
+# This should match addRuleButton.tooltip in styleinspector.dtd
+ruleView.contextmenu.addNewRule=Add new rule
 
 # LOCALIZATION NOTE (ruleView.contextmenu.addRule.accessKey): Access key for
 # the rule view context menu "Add rule" entry.
-ruleView.contextmenu.addRule.accessKey=R
+ruleView.contextmenu.addNewRule.accessKey=R
 
 # LOCALIZATION NOTE (computedView.contextmenu.selectAll): Text displayed in the
 # computed view context menu.
 computedView.contextmenu.selectAll=Select all
 
 # LOCALIZATION NOTE (computedView.contextmenu.selectAll.accessKey): Access key for
 # the computed view context menu "Select all" entry.
 computedView.contextmenu.selectAll.accessKey=A