Bug 745961 - Hard to find the clickable region for adding a new CSS property in the Style Inspector. r=paul
authorDave Camp <dcamp@mozilla.com>
Fri, 01 Jun 2012 15:13:48 -0700
changeset 95401 13d899f25454
parent 95400 85c153eea9d4
child 95478 5199196b65ec
push id817
push userdcamp@campd.org
push dateFri, 01 Jun 2012 22:14:18 +0000
treeherderfx-team@13d899f25454 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspaul
bugs745961
milestone15.0a1
Bug 745961 - Hard to find the clickable region for adding a new CSS property in the Style Inspector. r=paul
browser/devtools/styleinspector/CssRuleView.jsm
browser/devtools/styleinspector/styleinspector.css
browser/devtools/styleinspector/test/browser_ruleview_bug_703643_context_menu_copy.js
browser/themes/gnomestripe/devtools/csshtmltree.css
browser/themes/pinstripe/devtools/csshtmltree.css
browser/themes/winstripe/devtools/csshtmltree.css
--- a/browser/devtools/styleinspector/CssRuleView.jsm
+++ b/browser/devtools/styleinspector/CssRuleView.jsm
@@ -1346,19 +1346,22 @@ RuleEditor.prototype = {
       class: "ruleview-selector"
     });
 
     this.openBrace = createChild(header, "span", {
       class: "ruleview-ruleopen",
       textContent: " {"
     });
 
-    this.openBrace.addEventListener("click", function() {
-      this.newProperty();
-    }.bind(this), true);
+    code.addEventListener("click", function() {
+      let selection = this.doc.defaultView.getSelection();
+      if (selection.isCollapsed) {
+        this.newProperty();
+      }
+    }.bind(this), false);
 
     this.propertyList = createChild(code, "ul", {
       class: "ruleview-propertylist"
     });
 
     this.populate();
 
     this.closeBrace = createChild(code, "div", {
@@ -1407,16 +1410,21 @@ RuleEditor.prototype = {
 
   /**
    * Create a text input for a property name.  If a non-empty property
    * name is given, we'll create a real TextProperty and add it to the
    * rule.
    */
   newProperty: function RuleEditor_newProperty()
   {
+    // If we're already creating a new property, ignore this.
+    if (!this.closeBrace.hasAttribute("tabindex")) {
+      return;
+    }
+
     // While we're editing a new property, it doesn't make sense to
     // start a second new property editor, so disable focusing the
     // close brace for now.
     this.closeBrace.removeAttribute("tabindex");
 
     this.newPropItem = createChild(this.propertyList, "li", {
       class: "ruleview-property ruleview-newproperty",
     });
@@ -1517,35 +1525,55 @@ TextPropertyEditor.prototype = {
     this.enable.addEventListener("click", this._onEnableClicked, true);
 
     // Click to expand the computed properties of the text property.
     this.expander = createChild(this.element, "span", {
       class: "ruleview-expander"
     });
     this.expander.addEventListener("click", this._onExpandClicked, true);
 
+    this.nameContainer = createChild(this.element, "span", {
+      class: "ruleview-namecontainer"
+    });
+    this.nameContainer.addEventListener("click", function(aEvent) {
+      this.nameSpan.click();
+      aEvent.stopPropagation();
+    }.bind(this), false);
+
     // Property name, editable when focused.  Property name
     // is committed when the editor is unfocused.
-    this.nameSpan = createChild(this.element, "span", {
+    this.nameSpan = createChild(this.nameContainer, "span", {
       class: "ruleview-propertyname",
       tabindex: "0",
     });
+
     editableField({
       start: this._onStartEditing,
       element: this.nameSpan,
       done: this._onNameDone,
       advanceChars: ':'
     });
 
-    appendText(this.element, ": ");
+    appendText(this.nameContainer, ": ");
+
+    // Create a span that will hold the property and semicolon.
+    // Use this span to create a slightly larger click target
+    // for the value.
+    let propertyContainer = createChild(this.element, "span", {
+      class: "ruleview-propertycontainer"
+    });
+    propertyContainer.addEventListener("click", function(aEvent) {
+      this.valueSpan.click();
+      aEvent.stopPropagation();
+    }.bind(this), false);
 
     // Property value, editable when focused.  Changes to the
     // property value are applied as they are typed, and reverted
     // if the user presses escape.
-    this.valueSpan = createChild(this.element, "span", {
+    this.valueSpan = createChild(propertyContainer, "span", {
       class: "ruleview-propertyvalue",
       tabindex: "0",
     });
 
     editableField({
       start: this._onStartEditing,
       element: this.valueSpan,
       done: this._onValueDone,
@@ -1553,17 +1581,17 @@ TextPropertyEditor.prototype = {
     });
 
     // Save the initial value as the last committed value,
     // for restoring after pressing escape.
     this.committed = { name: this.prop.name,
                        value: this.prop.value,
                        priority: this.prop.priority };
 
-    appendText(this.element, ";");
+    appendText(propertyContainer, ";");
 
     this.warning = createChild(this.element, "div", {
       hidden: "",
       class: "ruleview-warning",
       title: CssLogic.l10n("rule.warning.title"),
     });
 
     // Holds the viewers for the computed properties.
@@ -1667,28 +1695,30 @@ TextPropertyEditor.prototype = {
     } else {
       this.expander.style.visibility = "hidden";
     }
   },
 
   /**
    * Handles clicks on the disabled property.
    */
-  _onEnableClicked: function TextPropertyEditor_onEnableClicked()
+  _onEnableClicked: function TextPropertyEditor_onEnableClicked(aEvent)
   {
     this.prop.setEnabled(this.enable.checked);
+    aEvent.stopPropagation();
   },
 
   /**
    * Handles clicks on the computed property expander.
    */
-  _onExpandClicked: function TextPropertyEditor_onExpandClicked()
+  _onExpandClicked: function TextPropertyEditor_onExpandClicked(aEvent)
   {
     this.expander.classList.toggle("styleinspector-open");
     this.computed.classList.toggle("styleinspector-open");
+    aEvent.stopPropagation();
   },
 
   /**
    * Called when the property name's inplace editor is closed.
    * Ignores the change if the user pressed escape, otherwise
    * commits it.
    *
    * @param {string} aValue
@@ -1806,22 +1836,23 @@ function editableField(aOptions)
  * @param DOMElement aElement
  *        The DOM element.
  * @param function aCallback
  *        Called when the editor is activated.
  */
 
 function editableItem(aElement, aCallback)
 {
-  aElement.addEventListener("click", function() {
+  aElement.addEventListener("click", function(evt) {
     let win = this.ownerDocument.defaultView;
     let selection = win.getSelection();
     if (selection.isCollapsed) {
       aCallback(aElement);
     }
+    evt.stopPropagation();
   }, false);
 
   // If focused by means other than a click, start editing by
   // pressing enter or space.
   aElement.addEventListener("keypress", function(evt) {
     if (evt.keyCode === Ci.nsIDOMKeyEvent.DOM_VK_RETURN ||
         evt.charCode === Ci.nsIDOMKeyEvent.DOM_VK_SPACE) {
       aCallback(aElement);
--- a/browser/devtools/styleinspector/styleinspector.css
+++ b/browser/devtools/styleinspector/styleinspector.css
@@ -40,8 +40,17 @@
 
 .ruleview-code {
   direction: ltr;
 }
 
 .ruleview-property:not(:hover) > .ruleview-enableproperty {
   visibility: hidden;
 }
+
+.ruleview-namecontainer {
+  cursor: text;
+}
+
+.ruleview-propertycontainer {
+  cursor: text;
+  padding-right: 15px;
+}
--- a/browser/devtools/styleinspector/test/browser_ruleview_bug_703643_context_menu_copy.js
+++ b/browser/devtools/styleinspector/test/browser_ruleview_bug_703643_context_menu_copy.js
@@ -223,25 +223,26 @@ function checkCopyPropertyValue()
     failedClipboard(expectedPattern, checkCopySelection);
   });
 }
 
 function checkCopySelection()
 {
   let contentDoc = ruleViewFrame().contentDocument;
   let props = contentDoc.querySelectorAll(".ruleview-property");
+  let values = contentDoc.querySelectorAll(".ruleview-propertycontainer");
 
   let range = document.createRange();
   range.setStart(props[0], 0);
-  range.setEnd(props[4], 8);
+  range.setEnd(values[4], 2);
 
   let selection = ruleViewFrame().contentWindow.getSelection();
   selection.addRange(range);
 
-  info("Checking that _boundCopy()  returns the correct" +
+  info("Checking that _boundCopy() returns the correct " +
     "clipboard value");
   let expectedPattern = "    margin: 10em;[\\r\\n]+" +
                         "    font-size: 14pt;[\\r\\n]+" +
                         "    font-family: helvetica,sans-serif;[\\r\\n]+" +
                         "    color: rgb\\(170, 170, 170\\);[\\r\\n]+" +
                         "}[\\r\\n]+" +
                         "html {[\\r\\n]+" +
                         "    color: rgb\\(0, 0, 0\\);[\\r\\n]*";
--- a/browser/themes/gnomestripe/devtools/csshtmltree.css
+++ b/browser/themes/gnomestripe/devtools/csshtmltree.css
@@ -169,17 +169,17 @@
   height: 12px;
 }
 
 .ruleview-ruleopen {
   -moz-padding-end: 5px;
 }
 
 .ruleview-ruleclose {
-  width: -moz-min-content;
+  cursor: text;
   padding-right: 20px;
 }
 
 .ruleview-propertylist {
   list-style: none;
   padding: 0;
   margin: 0;
 }
@@ -206,23 +206,23 @@
 
 .ruleview-newproperty {
   /* (enable checkbox width: 12px) + (expander width: 15px) */
   -moz-margin-start: 27px;
 }
 
 .ruleview-propertyname {
   padding: 1px 0;
-  cursor: text;
   color: #0060C0;
-  text-decoration: inherit;
 }
 
+.ruleview-namecontainer,
+.ruleview-propertycontainer,
+.ruleview-propertyname,
 .ruleview-propertyvalue {
-  cursor: text;
   text-decoration: inherit;
 }
 
 .ruleview-computedlist {
   list-style: none;
   padding: 0;
 }
 
@@ -242,8 +242,18 @@
 
 .ruleview-property {
   border-left: 2px solid transparent;
 }
 
 .ruleview-property[dirty] {
   border-left-color: #68E268;
 }
+
+.ruleview-namecontainer > .ruleview-propertyname,
+.ruleview-propertycontainer > .ruleview-propertyvalue {
+  border-bottom: 1px dotted transparent;
+}
+
+.ruleview-namecontainer:hover > .ruleview-propertyname,
+.ruleview-propertycontainer:hover > .ruleview-propertyvalue {
+  border-bottom-color: black;
+}
--- a/browser/themes/pinstripe/devtools/csshtmltree.css
+++ b/browser/themes/pinstripe/devtools/csshtmltree.css
@@ -171,17 +171,17 @@
   height: 12px;
 }
 
 .ruleview-ruleopen {
   -moz-padding-end: 5px;
 }
 
 .ruleview-ruleclose {
-  width: -moz-min-content;
+  cursor: text;
   padding-right: 20px;
 }
 
 .ruleview-propertylist {
   list-style: none;
   padding: 0;
   margin: 0;
 }
@@ -208,23 +208,23 @@
 
 .ruleview-newproperty {
   /* (enable checkbox width: 12px) + (expander width: 15px) */
   -moz-margin-start: 27px;
 }
 
 .ruleview-propertyname {
   padding: 1px 0;
-  cursor: text;
   color: #0060C0;
-  text-decoration: inherit;
 }
 
+.ruleview-namecontainer,
+.ruleview-propertycontainer,
+.ruleview-propertyname,
 .ruleview-propertyvalue {
-  cursor: text;
   text-decoration: inherit;
 }
 
 .ruleview-computedlist {
   list-style: none;
   padding: 0;
 }
 
@@ -244,8 +244,18 @@
 
 .ruleview-property {
   border-left: 2px solid transparent;
 }
 
 .ruleview-property[dirty] {
   border-left-color: #68E268;
 }
+
+.ruleview-namecontainer > .ruleview-propertyname,
+.ruleview-propertycontainer > .ruleview-propertyvalue {
+  border-bottom: 1px dotted transparent;
+}
+
+.ruleview-namecontainer:hover > .ruleview-propertyname,
+.ruleview-propertycontainer:hover > .ruleview-propertyvalue {
+  border-bottom-color: black;
+}
--- a/browser/themes/winstripe/devtools/csshtmltree.css
+++ b/browser/themes/winstripe/devtools/csshtmltree.css
@@ -170,17 +170,17 @@
   height: 12px;
 }
 
 .ruleview-ruleopen {
   -moz-padding-end: 5px;
 }
 
 .ruleview-ruleclose {
-  width: -moz-min-content;
+  cursor: text;
   padding-right: 20px;
 }
 
 .ruleview-propertylist {
   list-style: none;
   padding: 0;
   margin: 0;
 }
@@ -207,23 +207,23 @@
 
 .ruleview-newproperty {
   /* (enable checkbox width: 12px) + (expander width: 15px) */
   -moz-margin-start: 27px;
 }
 
 .ruleview-propertyname {
   padding: 1px 0;
-  cursor: text;
   color: #0060C0;
-  text-decoration: inherit;
 }
 
+.ruleview-namecontainer,
+.ruleview-propertycontainer,
+.ruleview-propertyname,
 .ruleview-propertyvalue {
-  cursor: text;
   text-decoration: inherit;
 }
 
 .ruleview-computedlist {
   list-style: none;
   padding: 0;
 }
 
@@ -243,8 +243,18 @@
 
 .ruleview-property {
   border-left: 2px solid transparent;
 }
 
 .ruleview-property[dirty] {
   border-left-color: #68E268;
 }
+
+.ruleview-namecontainer > .ruleview-propertyname,
+.ruleview-propertycontainer > .ruleview-propertyvalue {
+  border-bottom: 1px dotted transparent;
+}
+
+.ruleview-namecontainer:hover > .ruleview-propertyname,
+.ruleview-propertycontainer:hover > .ruleview-propertyvalue {
+  border-bottom-color: black;
+}