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 95477 13d899f25454
parent 95476 85c153eea9d4
child 95478 5199196b65ec
push id22817
push userdcamp@campd.org
push dateSat, 02 Jun 2012 04:40:57 +0000
treeherdermozilla-central@5199196b65ec [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspaul
bugs745961
milestone15.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 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;
+}