Bug 1198627 - Prevent errors when keyboard navigating inspector breadcrumbs; r=bgrins

--HG--
extra : commitid : BknFyMAS6eX
extra : rebase_source : 5383e8503c9656a45a12f1208f7c51c09043d32e

browser/devtools/inspector/breadcrumbs.js

@@ -237,7 +237,7 @@ HTMLBreadcrumbs.prototype = {
     // We make sure that the targeted node is selected
     // because we want to use the nodemenu that only works
     // for inspector.selection
-    this.selection.setNodeFront(node, "breadcrumbs");
+    this.navigateTo(node);
 
     // Build a list of extra menu items that will be appended at the end of the
     // inspector node context menu.
@@ -262,10 +262,10 @@ HTMLBreadcrumbs.prototype = {
         item.setAttribute("type", "radio");
         item.setAttribute("label", this.prettyPrintNodeAsText(nodes[i]));
 
-        let selection = this.selection;
+        let self = this;
         item.onmouseup = (function(node) {
           return function() {
-            selection.setNodeFront(node, "breadcrumbs");
+            self.navigateTo(node);
           };
         })(nodes[i]);
 
@@ -353,39 +353,37 @@ HTMLBreadcrumbs.prototype = {
    * @param {DOMEvent} event.
    */
   handleKeyPress: function(event) {
-    let node = null;
+    let navigate = promise.resolve(null);
-    this._keyPromise = this._keyPromise || promise.resolve(null);
 
     this._keyPromise = (this._keyPromise || promise.resolve(null)).then(() => {
       switch (event.keyCode) {
         case this.chromeWin.KeyEvent.DOM_VK_LEFT:
           if (this.currentIndex != 0) {
-            node = promise.resolve(this.nodeHierarchy[this.currentIndex - 1].node);
+            navigate = promise.resolve(
+              this.nodeHierarchy[this.currentIndex - 1].node);
           }
           break;
         case this.chromeWin.KeyEvent.DOM_VK_RIGHT:
           if (this.currentIndex < this.nodeHierarchy.length - 1) {
-            node = promise.resolve(this.nodeHierarchy[this.currentIndex + 1].node);
+            navigate = promise.resolve(
+              this.nodeHierarchy[this.currentIndex + 1].node);
           }
           break;
         case this.chromeWin.KeyEvent.DOM_VK_UP:
-          node = this.walker.previousSibling(this.selection.nodeFront, {
+          navigate = this.walker.previousSibling(this.selection.nodeFront, {
             whatToShow: Ci.nsIDOMNodeFilter.SHOW_ELEMENT
           });
           break;
         case this.chromeWin.KeyEvent.DOM_VK_DOWN:
-          node = this.walker.nextSibling(this.selection.nodeFront, {
+          navigate = this.walker.nextSibling(this.selection.nodeFront, {
             whatToShow: Ci.nsIDOMNodeFilter.SHOW_ELEMENT
           });
           break;
       }
 
-      return node.then((node) => {
+      return navigate.then(node => this.navigateTo(node));
-        if (node) {
-          this.selection.setNodeFront(node, "breadcrumbs");
-        }
-      });
     });
+
     event.preventDefault();
     event.stopPropagation();
   },
@@ -470,6 +468,14 @@ HTMLBreadcrumbs.prototype = {
     }
   },
 
+  navigateTo: function(node) {
+    if (node) {
+      this.selection.setNodeFront(node, "breadcrumbs");
+    } else {
+      this.inspector.emit("breadcrumbs-navigation-cancelled");
+    }
+  },
+
   /**
    * Build a button representing the node.
    * @param {NodeFront} node The node from the page.
@@ -490,7 +496,7 @@ HTMLBreadcrumbs.prototype = {
     };
 
     button.onBreadcrumbsClick = () => {
-      this.selection.setNodeFront(node, "breadcrumbs");
+      this.navigateTo(node);
     };
 
     button.onBreadcrumbsHover = () => {

browser/devtools/inspector/test/browser.ini

@@ -31,6 +31,7 @@ support-files =
 
 [browser_inspector_breadcrumbs.js]
 [browser_inspector_breadcrumbs_highlight_hover.js]
+[browser_inspector_breadcrumbs_keybinding.js]
 [browser_inspector_breadcrumbs_menu.js]
 [browser_inspector_breadcrumbs_mutations.js]
 [browser_inspector_delete-selected-node-01.js]

browser/devtools/inspector/test/browser_inspector_breadcrumbs_keybinding.js

@@ -0,0 +1,109 @@
+/* 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";
+
+// Test that the breadcrumbs keybindings work.
+
+const TEST_URI = TEST_URL_ROOT + "doc_inspector_breadcrumbs.html";
+const TEST_DATA = [{
+  desc: "Pressing left should select the parent <body>",
+  key: "VK_LEFT",
+  newSelection: "body"
+}, {
+  desc: "Pressing left again should select the parent <html>",
+  key: "VK_LEFT",
+  newSelection: "html"
+}, {
+  desc: "Pressing left again should stay on root <html>",
+  key: "VK_LEFT",
+  newSelection: "html"
+}, {
+  desc: "Pressing right should go down to <body>",
+  key: "VK_RIGHT",
+  newSelection: "body"
+}, {
+  desc: "Pressing right again should go down to #i2",
+  key: "VK_RIGHT",
+  newSelection: "#i2"
+}, {
+  desc: "Continue down to #i21",
+  key: "VK_RIGHT",
+  newSelection: "#i21"
+}, {
+  desc: "Continue down to #i211",
+  key: "VK_RIGHT",
+  newSelection: "#i211"
+}, {
+  desc: "Continue down to #i2111",
+  key: "VK_RIGHT",
+  newSelection: "#i2111"
+}, {
+  desc: "Pressing right once more should stay at leaf node #i2111",
+  key: "VK_RIGHT",
+  newSelection: "#i2111"
+}, {
+  desc: "Go back to #i211",
+  key: "VK_LEFT",
+  newSelection: "#i211"
+}, {
+  desc: "Go back to #i21",
+  key: "VK_LEFT",
+  newSelection: "#i21"
+}, {
+  desc: "Pressing down should move to next sibling #i22",
+  key: "VK_DOWN",
+  newSelection: "#i22"
+}, {
+  desc: "Pressing up should move to previous sibling #i21",
+  key: "VK_UP",
+  newSelection: "#i21"
+}, {
+  desc: "Pressing up again should stay on #i21 as there's no previous sibling",
+  key: "VK_UP",
+  newSelection: "#i21"
+}, {
+  desc: "Going back down to #i22",
+  key: "VK_DOWN",
+  newSelection: "#i22"
+}, {
+  desc: "Pressing down again should stay on #i22 as there's no next sibling",
+  key: "VK_DOWN",
+  newSelection: "#i22"
+}];
+
+add_task(function*() {
+  let {inspector} = yield openInspectorForURL(TEST_URI);
+
+  info("Selecting the test node");
+  yield selectNode("#i2", inspector);
+
+  info("Clicking on the corresponding breadcrumbs node to focus it");
+  let container = inspector.panelDoc.getElementById("inspector-breadcrumbs");
+
+  let button = container.querySelector("button[checked]");
+  button.click();
+
+  let currentSelection = "#id2";
+  for (let {desc, key, newSelection} of TEST_DATA) {
+    info(desc);
+
+    let onUpdated;
+    if (newSelection !== currentSelection) {
+      info("Expecting a new node to be selected");
+      onUpdated = inspector.once("breadcrumbs-updated");
+    } else {
+      info("Expecting the same node to remain selected");
+      onUpdated = inspector.once("breadcrumbs-navigation-cancelled");
+    }
+
+    EventUtils.synthesizeKey(key, {});
+    yield onUpdated;
+
+    let newNodeFront = yield getNodeFront(newSelection, inspector);
+    is(newNodeFront, inspector.selection.nodeFront,
+       "The current selection is correct");
+
+    currentSelection = newSelection;
+  }
+});