Bug 835808 - Navigate with arrow keys in computed view; r=harth

browser/devtools/styleinspector/computed-view.js

@@ -668,16 +668,13 @@ PropertyView.prototype = {
 
   /**
    * Returns the className that should be assigned to the propertyView.
-   *
    * @return string
    */
   get propertyHeaderClassName()
   {
     if (this.visible) {
-      this.tree._darkStripe = !this.tree._darkStripe;
-      let darkValue = this.tree._darkStripe ?
-                      "property-view theme-bg-darker" : "property-view";
-      return darkValue;
+      let isDark = this.tree._darkStripe = !this.tree._darkStripe;
+      return isDark ? "property-view theme-bg-darker" : "property-view";
     }
     return "property-view-hidden";
   },
@@ -690,49 +687,66 @@ PropertyView.prototype = {
   get propertyContentClassName()
   {
     if (this.visible) {
-      let darkValue = this.tree._darkStripe ?
-                      "property-content theme-bg-darker" : "property-content";
-      return darkValue;
+      let isDark = this.tree._darkStripe;
+      return isDark ? "property-content theme-bg-darker" : "property-content";
     }
     return "property-content-hidden";
   },
 
+  /**
+   * Build the markup for on computed style
+   * @return Element
+   */
   buildMain: function PropertyView_buildMain()
   {
     let doc = this.tree.styleDocument;
+    let onToggle = this.onStyleToggle.bind(this);
+
+    // Build the container element
     this.element = doc.createElementNS(HTML_NS, "div");
     this.element.setAttribute("class", this.propertyHeaderClassName);
 
-    this.matchedExpander = doc.createElementNS(HTML_NS, "div");
-    this.matchedExpander.className = "expander theme-twisty";
-    this.matchedExpander.setAttribute("tabindex", "0");
-    this.matchedExpander.addEventListener("click",
-      this.matchedExpanderClick.bind(this), false);
-    this.matchedExpander.addEventListener("keydown", function(aEvent) {
+    // Make it keyboard navigable
+    this.element.setAttribute("tabindex", "0");
+    this.element.addEventListener("keydown", function(aEvent) {
       let keyEvent = Ci.nsIDOMKeyEvent;
       if (aEvent.keyCode == keyEvent.DOM_VK_F1) {
         this.mdnLinkClick();
       }
       if (aEvent.keyCode == keyEvent.DOM_VK_RETURN ||
         aEvent.keyCode == keyEvent.DOM_VK_SPACE) {
-        this.matchedExpanderClick(aEvent);
+        onToggle(aEvent);
       }
     }.bind(this), false);
+
+    // Build the twisty expand/collapse
+    this.matchedExpander = doc.createElementNS(HTML_NS, "div");
+    this.matchedExpander.className = "expander theme-twisty";
+    this.matchedExpander.addEventListener("click", onToggle, false);
     this.element.appendChild(this.matchedExpander);
 
+    // Build the style name element
     this.nameNode = doc.createElementNS(HTML_NS, "div");
-    this.element.appendChild(this.nameNode);
     this.nameNode.setAttribute("class", "property-name theme-fg-color5");
+    // Reset its tabindex attribute otherwise, if an ellipsis is applied
+    // it will be reachable via TABing
+    this.nameNode.setAttribute("tabindex", "");
     this.nameNode.textContent = this.nameNode.title = this.name;
-    this.nameNode.addEventListener("click", function(aEvent) {
-      this.matchedExpander.focus();
-    }.bind(this), false);
+    // Make it hand over the focus to the container
+    this.nameNode.addEventListener("click", () => this.element.focus(), false);
+    this.element.appendChild(this.nameNode);
 
+    // Build the style value element
     this.valueNode = doc.createElementNS(HTML_NS, "div");
-    this.element.appendChild(this.valueNode);
     this.valueNode.setAttribute("class", "property-value theme-fg-color1");
+    // Reset its tabindex attribute otherwise, if an ellipsis is applied
+    // it will be reachable via TABing
+    this.valueNode.setAttribute("tabindex", "");
     this.valueNode.setAttribute("dir", "ltr");
     this.valueNode.textContent = this.valueNode.title = this.value;
+    // Make it hand over the focus to the container
+    this.valueNode.addEventListener("click", () => this.element.focus(), false);
+    this.element.appendChild(this.valueNode);
 
     return this.element;
   },
@@ -836,7 +850,7 @@ PropertyView.prototype = {
    * @param {Event} aEvent Used to determine the class name of the targets click
    * event.
    */
-  matchedExpanderClick: function PropertyView_matchedExpanderClick(aEvent)
+  onStyleToggle: function PropertyView_onStyleToggle(aEvent)
   {
     this.matchedExpanded = !this.matchedExpanded;
     this.refreshMatchedSelectors();

browser/devtools/styleinspector/test/Makefile.in

@@ -36,6 +36,7 @@ MOCHITEST_BROWSER_FILES = \
   browser_bug894376_css_value_completion_existing_property_value_pair.js \
   browser_ruleview_bug_902966_revert_value_on_ESC.js \
   browser_ruleview_pseudoelement.js \
+  browser_computedview_bug835808_keyboard_nav.js \
   head.js \
   $(NULL)
 

browser/devtools/styleinspector/test/browser_computedview_bug835808_keyboard_nav.js

@@ -0,0 +1,94 @@
+/* vim: set ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Tests that the style inspector works properly
+
+let doc, computedView, inspector;
+
+function test()
+{
+  waitForExplicitFinish();
+
+  gBrowser.selectedTab = gBrowser.addTab();
+  gBrowser.selectedBrowser.addEventListener("load", function onBrowserLoad(evt) {
+    gBrowser.selectedBrowser.removeEventListener("load", onBrowserLoad, true);
+    doc = content.document;
+    waitForFocus(createDocument, content);
+  }, true);
+
+  content.location = "data:text/html,computed view context menu test";
+}
+
+function createDocument()
+{
+  doc.body.innerHTML = '<style type="text/css"> ' +
+    'span { font-variant: small-caps; color: #000000; } ' +
+    '.nomatches {color: #ff0000;}</style> <div id="first" style="margin: 10em; ' +
+    'font-size: 14pt; font-family: helvetica, sans-serif; color: #AAA">\n' +
+    '<h1>Some header text</h1>\n' +
+    '<p id="salutation" style="font-size: 12pt">hi.</p>\n' +
+    '<p id="body" style="font-size: 12pt">I am a test-case. This text exists ' +
+    'solely to provide some things to <span style="color: yellow">' +
+    'highlight</span> and <span style="font-weight: bold">count</span> ' +
+    'style list-items in the box at right. If you are reading this, ' +
+    'you should go do something else instead. Maybe read a book. Or better ' +
+    'yet, write some test-cases for another bit of code. ' +
+    '<span style="font-style: italic">some text</span></p>\n' +
+    '<p id="closing">more text</p>\n' +
+    '<p>even more text</p>' +
+    '</div>';
+  doc.title = "Computed view keyboard navigation test";
+
+  openComputedView(startTests);
+}
+
+function startTests(aInspector, aComputedView)
+{
+  computedView = aComputedView;
+  inspector = aInspector;
+  testTabThrougStyles();
+}
+
+function endTests()
+{
+  computedView = inspector = doc = null;
+  gBrowser.removeCurrentTab();
+  finish();
+}
+
+function testTabThrougStyles()
+{
+  let span = doc.querySelector("span");
+
+  inspector.once("computed-view-refreshed", () => {
+    // Selecting the first computed style in the list
+    let firstStyle = computedView.styleDocument.querySelector(".property-view");
+    ok(firstStyle, "First computed style found in panel");
+    firstStyle.focus();
+
+    // Tab to select the 2nd style, press return
+    EventUtils.synthesizeKey("VK_TAB", {});
+    EventUtils.synthesizeKey("VK_RETURN", {});
+    inspector.once("computed-view-property-expanded", () => {
+      // Verify the 2nd style has been expanded
+      let secondStyleSelectors = computedView.styleDocument.querySelectorAll(
+        ".property-content .matchedselectors")[1];
+      ok(secondStyleSelectors.childNodes.length > 0, "Matched selectors expanded");
+
+      // Tab back up and test the same thing, with space
+      EventUtils.synthesizeKey("VK_TAB", {shiftKey: true});
+      EventUtils.synthesizeKey("VK_SPACE", {});
+      inspector.once("computed-view-property-expanded", () => {
+        // Verify the 1st style has been expanded too
+        let firstStyleSelectors = computedView.styleDocument.querySelectorAll(
+          ".property-content .matchedselectors")[0];
+        ok(firstStyleSelectors.childNodes.length > 0, "Matched selectors expanded");
+
+        endTests();
+      });
+    });
+  });
+
+  inspector.selection.setNode(span);
+}

browser/themes/linux/devtools/computedview.css

@@ -45,6 +45,7 @@ body {
   overflow-x: hidden;
   text-overflow: ellipsis;
   white-space: nowrap;
+  outline: 0;
 }
 
 .property-value {
@@ -58,6 +59,7 @@ body {
   background-size: 5px 8px;
   background-position: 2px center;
   padding-left: 10px;
+  outline: 0;
 }
 
 .other-property-value {

browser/themes/osx/devtools/computedview.css

@@ -63,6 +63,7 @@ body {
   overflow-x: hidden;
   text-overflow: ellipsis;
   white-space: nowrap;
+  outline: 0;
 }
 
 .property-value {
@@ -76,6 +77,7 @@ body {
   background-size: 5px 8px;
   background-position: 2px center;
   padding-left: 10px;
+  outline: 0;
 }
 
 .other-property-value {

browser/themes/windows/devtools/computedview.css

@@ -63,6 +63,7 @@ body {
   overflow-x: hidden;
   text-overflow: ellipsis;
   white-space: nowrap;
+  outline: 0;
 }
 
 .property-value {
@@ -76,6 +77,7 @@ body {
   background-size: 5px 8px;
   background-position: 2px center;
   padding-left: 10px;
+  outline: 0;
 }
 
 .other-property-value {