Bug 1264907 - Don't show the firstChild of the current selection in breadcrumbs; r=jdescottes
authorPatrick Brosset <pbrosset@mozilla.com>
Fri, 15 Apr 2016 12:03:33 +0200
changeset 336110 6191f8b4e99d5096c0060113ad71d6c4c5b7116b
parent 336109 e62b981d0cbe620420b0c6fb7af38a07be913afb
child 336111 55e56996c6cc693fe7dbd81581d8607def3c4280
push id6249
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 13:59:36 +0000
treeherdermozilla-beta@bad9d4f5bf7e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdescottes
bugs1264907
milestone49.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 1264907 - Don't show the firstChild of the current selection in breadcrumbs; r=jdescottes The breadcrumbs widget used to have a feature where it would show the first child of the current selection even the DOM tree hadn't been expanded that far yet. This was to allow keyboard navigating the DOM through the breadcrumbs. The breadcrumbs is a very rarely used widget and this code was unnecessarily making things complex. It was decided that this feature would be removed. Instead, the breadcrumbs now act as simple linear elements in a toolbar and you can keyboard navigate them with LEFT/RIGHT only. TAB/shift-TAB simply go in/out of the breadcrumbs widget. MozReview-Commit-ID: BmcaLnVBOBn
.eslintignore
devtools/client/inspector/breadcrumbs.js
devtools/client/inspector/fonts/test/head.js
devtools/client/inspector/markup/test/browser_markup_remove_xul_attributes.js
devtools/client/inspector/test/browser_inspector_breadcrumbs.js
devtools/client/inspector/test/browser_inspector_breadcrumbs_keybinding.js
devtools/client/inspector/test/browser_inspector_breadcrumbs_keyboard_trap.js
devtools/client/inspector/test/browser_inspector_breadcrumbs_mutations.js
devtools/client/inspector/test/browser_inspector_keyboard-shortcuts.js
devtools/client/inspector/test/browser_inspector_search-06.js
--- a/.eslintignore
+++ b/.eslintignore
@@ -83,16 +83,17 @@ devtools/client/eyedropper/**
 devtools/client/framework/**
 # devtools/client/inspector/shared/*.js files are eslint-clean, so they aren't
 # included in the ignore list.
 devtools/client/inspector/computed/**
 devtools/client/inspector/fonts/**
 devtools/client/inspector/shared/test/**
 devtools/client/inspector/test/**
 devtools/client/inspector/*.js
+!devtools/client/inspector/breadcrumbs.js
 devtools/client/jsonview/lib/**
 devtools/client/memory/**
 devtools/client/netmonitor/test/**
 devtools/client/netmonitor/har/test/**
 devtools/client/performance/**
 devtools/client/projecteditor/**
 devtools/client/promisedebugger/**
 devtools/client/responsivedesign/**
--- a/devtools/client/inspector/breadcrumbs.js
+++ b/devtools/client/inspector/breadcrumbs.js
@@ -1,39 +1,27 @@
 /* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set ft=javascript ts=2 et sw=2 tw=80: */
 /* 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/. */
 
 "use strict";
 
-const {Cu, Ci} = require("chrome");
-Cu.import("resource://gre/modules/XPCOMUtils.jsm");
+const {Ci} = require("chrome");
 const Services = require("Services");
 const promise = require("promise");
 const FocusManager = Services.focus;
 const {waitForTick} = require("devtools/shared/DevToolsUtils");
 
 const ENSURE_SELECTION_VISIBLE_DELAY_MS = 50;
 const ELLIPSIS = Services.prefs.getComplexValue(
     "intl.ellipsis",
     Ci.nsIPrefLocalizedString).data;
 const MAX_LABEL_LENGTH = 40;
-const LOW_PRIORITY_ELEMENTS = {
-  "HEAD": true,
-  "BASE": true,
-  "BASEFONT": true,
-  "ISINDEX": true,
-  "LINK": true,
-  "META": true,
-  "SCRIPT": true,
-  "STYLE": true,
-  "TITLE": true
-};
 
 /**
  * Display the ancestors of the current node and its children.
  * Only one "branch" of children are displayed (only one line).
  *
  * FIXME: Bug 822388 - Use the BreadcrumbsWidget in the Inspector.
  *
  * Mechanism:
@@ -106,44 +94,17 @@ HTMLBreadcrumbs.prototype = {
     this.selection.on("new-node-front", this.update);
     this.selection.on("pseudoclass", this.updateSelectors);
     this.selection.on("attribute-changed", this.updateSelectors);
     this.inspector.on("markupmutation", this.update);
     this.update();
   },
 
   /**
-   * Include in a promise's then() chain to reject the chain
-   * when the breadcrumbs' selection has changed while the promise
-   * was outstanding.
-   */
-  selectionGuard: function () {
-    let selection = this.selection.nodeFront;
-    return result => {
-      if (selection != this.selection.nodeFront) {
-        return promise.reject("selection-changed");
-      }
-      return result;
-    };
-  },
 
-  /**
-   * Warn if rejection was caused by selection change, print an error otherwise.
-   * @param {Error} err
-   */
-  selectionGuardEnd: function (err) {
-    // If the error is selection-changed, this is expected, the selection
-    // changed while we were waiting for a promise to resolve, so there's no
-    // need to proceed with the current update, and we should be silent.
-    if (err !== "selection-changed") {
-      console.error(err);
-    }
-  },
-
-  /**
    * Build a string that represents the node: tagName#id.class1.class2.
    * @param {NodeFront} node The node to pretty-print
    * @return {String}
    */
   prettyPrintNodeAsText: function (node) {
     let text = node.tagName.toLowerCase();
     if (node.isPseudoElement) {
       text = node.isBeforePseudoElement ? "::before" : "::after";
@@ -294,88 +255,85 @@ HTMLBreadcrumbs.prototype = {
    * On mouse leave, make sure to unhighlight.
    * @param {DOMEvent} event.
    */
   handleMouseLeave: function (event) {
     this.inspector.toolbox.highlighterUtils.unhighlight();
   },
 
   /**
-   * On key press, navigate the node hierarchy.
+   * On keypress, navigate through the list of breadcrumbs with the left/right
+   * arrow keys.
    * @param {DOMEvent} event.
    */
   handleKeyPress: function (event) {
-    let navigate = promise.resolve(null);
+    let win = this.chromeWin;
+    let {keyCode, shiftKey, metaKey, ctrlKey, altKey} = event;
 
-    this._keyPromise = (this._keyPromise || promise.resolve(null)).then(() => {
-      switch (event.keyCode) {
-        case this.chromeWin.KeyEvent.DOM_VK_LEFT:
-          if (this.currentIndex != 0) {
-            navigate = promise.resolve(
-              this.nodeHierarchy[this.currentIndex - 1].node);
-          }
-          break;
-        case this.chromeWin.KeyEvent.DOM_VK_RIGHT:
-          if (this.currentIndex < this.nodeHierarchy.length - 1) {
-            navigate = promise.resolve(
-              this.nodeHierarchy[this.currentIndex + 1].node);
-          }
-          break;
-        case this.chromeWin.KeyEvent.DOM_VK_UP:
-          navigate = this.walker.previousSibling(this.selection.nodeFront, {
-            whatToShow: Ci.nsIDOMNodeFilter.SHOW_ELEMENT
-          });
-          break;
-        case this.chromeWin.KeyEvent.DOM_VK_DOWN:
-          navigate = this.walker.nextSibling(this.selection.nodeFront, {
-            whatToShow: Ci.nsIDOMNodeFilter.SHOW_ELEMENT
-          });
-          break;
-        case this.chromeWin.KeyEvent.DOM_VK_TAB:
-          // Tabbing when breadcrumbs or its contents are focused should move
-          // focus to next/previous focusable element relative to breadcrumbs
-          // themselves.
-          let elm, type;
-          if (event.shiftKey) {
-            elm = this.container;
-            type = FocusManager.MOVEFOCUS_BACKWARD;
-          } else {
-            // To move focus to next element following the breadcrumbs, relative
-            // element needs to be the last element in breadcrumbs' subtree.
-            let last = this.container.lastChild;
-            while (last && last.lastChild) {
-              last = last.lastChild;
-            }
-            elm = last;
-            type = FocusManager.MOVEFOCUS_FORWARD;
-          }
-          FocusManager.moveFocus(this.chromeWin, elm, type, 0);
-          break;
-      }
+    // Only handle left, right, tab and shift tab, let anything else bubble up
+    // so native shortcuts work.
+    let hasModifier = metaKey || ctrlKey || altKey || shiftKey;
+    let isLeft = keyCode === win.KeyEvent.DOM_VK_LEFT && !hasModifier;
+    let isRight = keyCode === win.KeyEvent.DOM_VK_RIGHT && !hasModifier;
+    let isTab = keyCode === win.KeyEvent.DOM_VK_TAB && !hasModifier;
+    let isShiftTab = keyCode === win.KeyEvent.DOM_VK_TAB && shiftKey &&
+                     !metaKey && !ctrlKey && !altKey;
 
-      return navigate.then(node => this.navigateTo(node));
-    });
+    if (!isLeft && !isRight && !isTab && !isShiftTab) {
+      return;
+    }
 
     event.preventDefault();
     event.stopPropagation();
+
+    this.keyPromise = (this.keyPromise || promise.resolve(null)).then(() => {
+      if (isLeft && this.currentIndex != 0) {
+        let node = this.nodeHierarchy[this.currentIndex - 1].node;
+        return this.selection.setNodeFront(node, "breadcrumbs");
+      } else if (isRight && this.currentIndex < this.nodeHierarchy.length - 1) {
+        let node = this.nodeHierarchy[this.currentIndex + 1].node;
+        return this.selection.setNodeFront(node, "breadcrumbs");
+      } else if (isTab || isShiftTab) {
+        // Tabbing when breadcrumbs or its contents are focused should move
+        // focus to next/previous focusable element relative to breadcrumbs
+        // themselves.
+        let elm, type;
+        if (shiftKey) {
+          elm = this.container;
+          type = FocusManager.MOVEFOCUS_BACKWARD;
+        } else {
+          // To move focus to next element following the breadcrumbs, relative
+          // element needs to be the last element in breadcrumbs' subtree.
+          let last = this.container.lastChild;
+          while (last && last.lastChild) {
+            last = last.lastChild;
+          }
+          elm = last;
+          type = FocusManager.MOVEFOCUS_FORWARD;
+        }
+        FocusManager.moveFocus(win, elm, type, 0);
+      }
+
+      return null;
+    });
   },
 
   /**
    * Remove nodes and clean up.
    */
   destroy: function () {
     this.selection.off("new-node-front", this.update);
     this.selection.off("pseudoclass", this.updateSelectors);
     this.selection.off("attribute-changed", this.updateSelectors);
     this.inspector.off("markupmutation", this.update);
 
     this.container.removeEventListener("underflow",
-        this.onscrollboxreflow, false);
+      this.onscrollboxreflow, false);
     this.container.removeEventListener("overflow",
-        this.onscrollboxreflow, false);
+      this.onscrollboxreflow, false);
     this.container.removeEventListener("click", this, true);
     this.container.removeEventListener("keypress", this, true);
     this.container.removeEventListener("mouseover", this, true);
     this.container.removeEventListener("mouseleave", this, true);
     this.container.removeEventListener("focus", this, true);
 
     this.empty();
     this.separators.remove();
@@ -437,24 +395,16 @@ HTMLBreadcrumbs.prototype = {
    */
   cutAfter: function (index) {
     while (this.nodeHierarchy.length > (index + 1)) {
       let toRemove = this.nodeHierarchy.pop();
       this.container.removeChild(toRemove.button);
     }
   },
 
-  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.
    * @return {DOMNode} The <button> for this node.
    */
   buildButton: function (node) {
     let button = this.chromeDoc.createElement("button");
     button.appendChild(this.prettyPrintNodeAsXUL(node));
@@ -469,17 +419,17 @@ HTMLBreadcrumbs.prototype = {
       }
     };
 
     button.onclick = () => {
       button.focus();
     };
 
     button.onBreadcrumbsClick = () => {
-      this.navigateTo(node);
+      this.selection.setNodeFront(node, "breadcrumbs");
     };
 
     button.onBreadcrumbsHover = () => {
       this.inspector.toolbox.highlighterUtils.highlightNodeFront(node);
     };
 
     return button;
   },
@@ -508,93 +458,32 @@ HTMLBreadcrumbs.prototype = {
         });
       }
       node = node.parentNode();
     }
     this.container.appendChild(fragment, this.container.firstChild);
   },
 
   /**
-   * Get a child of a node that can be displayed in the breadcrumbs and that is
-   * probably visible. See LOW_PRIORITY_ELEMENTS.
-   * @param {NodeFront} node The parent node.
-   * @return {Promise} Resolves to the NodeFront.
-   */
-  getInterestingFirstNode: function (node) {
-    let deferred = promise.defer();
-
-    let fallback = null;
-    let lastNode = null;
-
-    let moreChildren = () => {
-      this.walker.children(node, {
-        start: lastNode,
-        maxNodes: 10,
-        whatToShow: Ci.nsIDOMNodeFilter.SHOW_ELEMENT
-      }).then(this.selectionGuard()).then(response => {
-        for (let childNode of response.nodes) {
-          if (!(childNode.tagName in LOW_PRIORITY_ELEMENTS)) {
-            deferred.resolve(childNode);
-            return;
-          }
-          if (!fallback) {
-            fallback = childNode;
-          }
-          lastNode = childNode;
-        }
-        if (response.hasLast) {
-          deferred.resolve(fallback);
-          return;
-        }
-        moreChildren();
-      }).catch(this.selectionGuardEnd);
-    };
-
-    moreChildren();
-    return deferred.promise;
-  },
-
-  /**
    * Find the "youngest" ancestor of a node which is already in the breadcrumbs.
    * @param {NodeFront} node.
    * @return {Number} Index of the ancestor in the cache, or -1 if not found.
    */
   getCommonAncestor: function (node) {
     while (node) {
       let idx = this.indexOf(node);
       if (idx > -1) {
         return idx;
       }
       node = node.parentNode();
     }
     return -1;
   },
 
   /**
-   * Make sure that the latest node in the breadcrumbs is not the selected node
-   * if the selected node still has children.
-   * @return {Promise}
-   */
-  ensureFirstChild: function () {
-    // If the last displayed node is the selected node
-    if (this.currentIndex == this.nodeHierarchy.length - 1) {
-      let node = this.nodeHierarchy[this.currentIndex].node;
-      return this.getInterestingFirstNode(node).then(child => {
-        // If the node has a child and we've not been destroyed in the meantime
-        if (child && !this.isDestroyed) {
-          // Show this child
-          this.expand(child);
-        }
-      });
-    }
-
-    return waitForTick().then(() => true);
-  },
-
-  /**
    * Ensure the selected node is visible.
    */
   scroll: function () {
     // FIXME bug 684352: make sure its immediate neighbors are visible too.
 
     let scrollbox = this.container;
     let element = this.nodeHierarchy[this.currentIndex].button;
 
@@ -645,21 +534,19 @@ HTMLBreadcrumbs.prototype = {
   _hasInterestingMutations: function (mutations) {
     if (!mutations || !mutations.length) {
       return false;
     }
 
     for (let {type, added, removed, target, attributeName} of mutations) {
       if (type === "childList") {
         // Only interested in childList mutations if the added or removed
-        // nodes are currently displayed, or if it impacts the last element in
-        // the breadcrumbs.
+        // nodes are currently displayed.
         return added.some(node => this.indexOf(node) > -1) ||
-               removed.some(node => this.indexOf(node) > -1) ||
-               this.indexOf(target) === this.nodeHierarchy.length - 1;
+               removed.some(node => this.indexOf(node) > -1);
       } else if (type === "attributes" && this.indexOf(target) > -1) {
         // Only interested in attributes mutations if the target is
         // currently displayed, and the attribute is either id or class.
         return attributeName === "class" || attributeName === "id";
       }
     }
 
     // Catch all return in case the mutations array was empty, or in case none
@@ -724,28 +611,19 @@ HTMLBreadcrumbs.prototype = {
       this.expand(this.selection.nodeFront);
 
       // we select the current node button
       idx = this.indexOf(this.selection.nodeFront);
       this.setCursor(idx);
     }
 
     let doneUpdating = this.inspector.updating("breadcrumbs");
-    // Add the first child of the very last node of the breadcrumbs if possible.
-    this.ensureFirstChild().then(this.selectionGuard()).then(() => {
-      if (this.isDestroyed) {
-        return null;
-      }
 
-      this.updateSelectors();
+    this.updateSelectors();
 
-      // Make sure the selected node and its neighbours are visible.
-      this.scroll();
-      return waitForTick().then(() => {
-        this.inspector.emit("breadcrumbs-updated", this.selection.nodeFront);
-        doneUpdating();
-      });
-    }).catch(err => {
-      doneUpdating(this.selection.nodeFront);
-      this.selectionGuardEnd(err);
+    // Make sure the selected node and its neighbours are visible.
+    this.scroll();
+    waitForTick().then(() => {
+      this.inspector.emit("breadcrumbs-updated", this.selection.nodeFront);
+      doneUpdating();
     });
   }
 };
--- a/devtools/client/inspector/fonts/test/head.js
+++ b/devtools/client/inspector/fonts/test/head.js
@@ -5,43 +5,47 @@
 "use strict";
 
 // Import the inspector's head.js first (which itself imports shared-head.js).
 Services.scriptloader.loadSubScript(
   "chrome://mochitests/content/browser/devtools/client/inspector/test/head.js",
   this);
 
 Services.prefs.setBoolPref("devtools.fontinspector.enabled", true);
+Services.prefs.setCharPref("devtools.inspector.activeSidebar", "fontinspector");
 registerCleanupFunction(() => {
   Services.prefs.clearUserPref("devtools.fontinspector.enabled");
 });
 
 /**
+ * The font-inspector doesn't participate in the inspector's update mechanism
+ * (i.e. it doesn't call inspector.updating() when updating), so simply calling
+ * the default selectNode isn't enough to guaranty that the panel has finished
+ * updating. We also need to wait for the fontinspector-updated event.
+ */
+var _selectNode = selectNode;
+selectNode = function* (node, inspector, reason) {
+  let onUpdated = inspector.once("fontinspector-updated");
+  yield _selectNode(node, inspector, reason);
+  yield onUpdated;
+};
+
+/**
  * Adds a new tab with the given URL, opens the inspector and selects the
  * font-inspector tab.
  * @return {Promise} resolves to a {toolbox, inspector, view} object
  */
 var openFontInspectorForURL = Task.async(function*(url) {
   yield addTab(url);
-  let {toolbox, inspector} = yield openInspectorSidebarTab("fontinspector");
+  let {toolbox, inspector} = yield openInspector();
 
-  /**
-   * Call selectNode to trigger font-inspector update so that we don't timeout
-   * if following conditions hold
-   * a) the initial 'fontinspector-updated' was emitted while we were waiting
-   *    for openInspector to resolve
-   * b) the font-inspector tab was selected by default which means the call to
-   *    select will not trigger another update.
-   *
-   * selectNode calls setNodeFront which always emits 'new-node' which calls
-   * FontInspector.update that emits the 'fontinspector-updated' event.
-   */
-  let onUpdated = inspector.once("fontinspector-updated");
+  // Call selectNode again here to force a fontinspector update since we don't
+  // know if the fontinspector-updated event has been sent while the inspector
+  // was being opened or not.
   yield selectNode("body", inspector);
-  yield onUpdated;
 
   return {
     toolbox,
     inspector,
     view: inspector.fontInspector
   };
 });
 
--- a/devtools/client/inspector/markup/test/browser_markup_remove_xul_attributes.js
+++ b/devtools/client/inspector/markup/test/browser_markup_remove_xul_attributes.js
@@ -13,18 +13,16 @@ add_task(function* () {
   let {inspector, testActor} = yield openInspectorForURL(TEST_URL);
 
   let panelFront = yield getNodeFront("#test", inspector);
   ok(panelFront.hasAttribute("id"),
      "panelFront has id attribute in the beginning");
 
   info("Removing panel's id attribute");
   let onMutation = inspector.once("markupmutation");
-  let onInspectorUpdated = inspector.once("inspector-updated");
   yield testActor.removeAttribute("#test", "id");
 
-  info("Waiting for markupmutation and inspector-updated");
+  info("Waiting for markupmutation");
   yield onMutation;
-  yield onInspectorUpdated;
 
   is(panelFront.hasAttribute("id"), false,
      "panelFront doesn't have id attribute anymore");
 });
--- a/devtools/client/inspector/test/browser_inspector_breadcrumbs.js
+++ b/devtools/client/inspector/test/browser_inspector_breadcrumbs.js
@@ -3,25 +3,25 @@
    http://creativecommons.org/publicdomain/zero/1.0/ */
 "use strict";
 
 // Test that the breadcrumbs widget content is correct.
 
 const TEST_URI = URL_ROOT + "doc_inspector_breadcrumbs.html";
 const NODES = [
   {selector: "#i1111", result: "i1 i11 i111 i1111"},
-  {selector: "#i22", result: "i2 i22 i221"},
+  {selector: "#i22", result: "i2 i22"},
   {selector: "#i2111", result: "i2 i21 i211 i2111"},
   {selector: "#i21", result: "i2 i21 i211 i2111"},
   {selector: "#i22211", result: "i2 i22 i222 i2221 i22211"},
   {selector: "#i22", result: "i2 i22 i222 i2221 i22211"},
-  {selector: "#i3", result: "i3 i31"},
+  {selector: "#i3", result: "i3"},
 ];
 
-add_task(function*() {
+add_task(function* () {
   let { inspector } = yield openInspectorForURL(TEST_URI);
   let container = inspector.panelDoc.getElementById("inspector-breadcrumbs");
 
   for (let node of NODES) {
     info("Testing node " + node.selector);
 
     info("Selecting node and waiting for breadcrumbs to update");
     let breadcrumbsUpdated = inspector.once("breadcrumbs-updated");
@@ -49,26 +49,28 @@ add_task(function*() {
     is(labelId.textContent, "#" + id,
       "Node #" + node.selector + ": selection matches");
   }
 
   yield testPseudoElements(inspector, container);
 });
 
 function* testPseudoElements(inspector, container) {
-  info ("Checking for pseudo elements");
+  info("Checking for pseudo elements");
 
   let pseudoParent = yield getNodeFront("#pseudo-container", inspector);
   let children = yield inspector.walker.children(pseudoParent);
-  is (children.nodes.length, 2, "Pseudo children returned from walker");
+  is(children.nodes.length, 2, "Pseudo children returned from walker");
 
   let beforeElement = children.nodes[0];
   let breadcrumbsUpdated = inspector.once("breadcrumbs-updated");
   yield selectNode(beforeElement, inspector);
   yield breadcrumbsUpdated;
-  is(container.childNodes[3].textContent, "::before", "::before shows up in breadcrumb");
+  is(container.childNodes[3].textContent, "::before",
+     "::before shows up in breadcrumb");
 
   let afterElement = children.nodes[1];
   breadcrumbsUpdated = inspector.once("breadcrumbs-updated");
   yield selectNode(afterElement, inspector);
   yield breadcrumbsUpdated;
-  is(container.childNodes[3].textContent, "::after", "::before shows up in breadcrumb");
+  is(container.childNodes[3].textContent, "::after",
+     "::before shows up in breadcrumb");
 }
--- a/devtools/client/inspector/test/browser_inspector_breadcrumbs_keybinding.js
+++ b/devtools/client/inspector/test/browser_inspector_breadcrumbs_keybinding.js
@@ -10,96 +10,55 @@ 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>",
+  desc: "Pressing left again should stay on <html>, it's the first element",
   key: "VK_LEFT",
   newSelection: "html"
 }, {
-  desc: "Pressing right should go down to <body>",
+  desc: "Pressing right should go to <body>",
   key: "VK_RIGHT",
   newSelection: "body"
 }, {
-  desc: "Pressing right again should go down to #i2",
+  desc: "Pressing right again should go 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",
+  desc: "Pressing right again should stay on #i2, it's the last element",
   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"
+  newSelection: "#i2"
 }];
 
-add_task(function*() {
+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 the selection will change, wait for the breadcrumb to update,
+    // otherwise continue.
+    let onUpdated = null;
     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");
--- a/devtools/client/inspector/test/browser_inspector_breadcrumbs_keyboard_trap.js
+++ b/devtools/client/inspector/test/browser_inspector_breadcrumbs_keyboard_trap.js
@@ -26,32 +26,34 @@ const TEST_DATA = [
   },
   {
     desc: "Move the focus back to the breadcrumbs",
     focused: true,
     key: "VK_TAB",
     options: { shiftKey: true }
   },
   {
-    desc: "Move the focus back away from breadcrumbs to a previous focusable element",
+    desc: "Move the focus back away from breadcrumbs to a previous focusable " +
+          "element",
     focused: false,
     key: "VK_TAB",
     options: { shiftKey: true }
   },
   {
     desc: "Move the focus back to the breadcrumbs",
     focused: true,
     key: "VK_TAB",
     options: { }
   }
 ];
 
-add_task(function*() {
+add_task(function* () {
   let { toolbox, inspector } = yield openInspectorForURL(TEST_URL);
   let doc = inspector.panelDoc;
+  let {breadcrumbs} = inspector;
 
   yield selectNode("#i2", inspector);
 
   info("Clicking on the corresponding breadcrumbs node to focus it");
   let container = doc.getElementById("inspector-breadcrumbs");
 
   let button = container.querySelector("button[checked]");
   let onHighlight = toolbox.once("node-highlight");
@@ -59,21 +61,19 @@ add_task(function*() {
   yield onHighlight;
 
   // Ensure a breadcrumb is focused.
   is(doc.activeElement, button, "Focus is on selected breadcrumb");
 
   for (let { desc, focused, key, options } of TEST_DATA) {
     info(desc);
 
-    let onUpdated;
-    if (!focused) {
-      onUpdated = inspector.once("breadcrumbs-navigation-cancelled");
-    }
     EventUtils.synthesizeKey(key, options);
+    // Wait until the keyPromise promise resolves.
+    yield breadcrumbs.keyPromise;
+
     if (focused) {
       is(doc.activeElement, button, "Focus is on selected breadcrumb");
     } else {
-      yield onUpdated;
       ok(!containsFocus(doc, container), "Focus is outside of breadcrumbs");
     }
   }
 });
--- a/devtools/client/inspector/test/browser_inspector_breadcrumbs_mutations.js
+++ b/devtools/client/inspector/test/browser_inspector_breadcrumbs_mutations.js
@@ -18,133 +18,141 @@ const TEST_URI = URL_ROOT + "doc_inspect
 //   actual test case, i.e, mutates the content DOM to cause the breadcrumbs
 //   to refresh, or not.
 // - shouldRefresh {Boolean} Once the `run` function has completed, and the test
 //   has detected that the page has changed, this boolean instructs the test to
 //   verify if the breadcrumbs has refreshed or not.
 // - output {Array} A list of strings for the text that should be found in each
 //   button after the test has run.
 const TEST_DATA = [{
-  desc: "Adding a child at the end of the chain should refresh and show it",
-  setup: function*(inspector) {
+  desc: "Adding a child at the end of the chain shouldn't change anything",
+  setup: function* (inspector) {
     yield selectNode("#i1111", inspector);
   },
-  run: function*({walker, selection}) {
+  run: function* ({walker, selection}) {
     yield walker.setInnerHTML(selection.nodeFront, "<b>test</b>");
   },
-  shouldRefresh: true,
-  output: ["html", "body", "article#i1", "div#i11", "div#i111", "div#i1111", "b"]
+  shouldRefresh: false,
+  output: ["html", "body", "article#i1", "div#i11", "div#i111", "div#i1111"]
 }, {
   desc: "Updating an ID to an displayed element should refresh",
-  setup: function*() {},
-  run: function*({walker}) {
+  setup: function* () {},
+  run: function* ({walker}) {
     let node = yield walker.querySelector(walker.rootNode, "#i1");
     yield node.modifyAttributes([{
       attributeName: "id",
       newValue: "i1-changed"
     }]);
   },
   shouldRefresh: true,
-  output: ["html", "body", "article#i1-changed", "div#i11", "div#i111", "div#i1111", "b"]
+  output: ["html", "body", "article#i1-changed", "div#i11", "div#i111",
+           "div#i1111"]
 }, {
   desc: "Updating an class to a displayed element should refresh",
-  setup: function*() {},
-  run: function*({walker}) {
+  setup: function* () {},
+  run: function* ({walker}) {
     let node = yield walker.querySelector(walker.rootNode, "body");
     yield node.modifyAttributes([{
       attributeName: "class",
       newValue: "test-class"
     }]);
   },
   shouldRefresh: true,
-  output: ["html", "body.test-class", "article#i1-changed", "div#i11", "div#i111", "div#i1111", "b"]
+  output: ["html", "body.test-class", "article#i1-changed", "div#i11",
+           "div#i111", "div#i1111"]
 }, {
-  desc: "Updating a non id/class attribute to a displayed element should not refresh",
-  setup: function*() {},
-  run: function*({walker}) {
+  desc: "Updating a non id/class attribute to a displayed element should not " +
+        "refresh",
+  setup: function* () {},
+  run: function* ({walker}) {
     let node = yield walker.querySelector(walker.rootNode, "#i11");
     yield node.modifyAttributes([{
       attributeName: "name",
       newValue: "value"
     }]);
   },
   shouldRefresh: false,
-  output: ["html", "body.test-class", "article#i1-changed", "div#i11", "div#i111", "div#i1111", "b"]
+  output: ["html", "body.test-class", "article#i1-changed", "div#i11",
+           "div#i111", "div#i1111"]
 }, {
   desc: "Moving a child in an element that's not displayed should not refresh",
-  setup: function*() {},
-  run: function*({walker}) {
+  setup: function* () {},
+  run: function* ({walker}) {
     // Re-append #i1211 as a last child of #i2.
     let parent = yield walker.querySelector(walker.rootNode, "#i2");
     let child = yield walker.querySelector(walker.rootNode, "#i211");
     yield walker.insertBefore(child, parent);
   },
   shouldRefresh: false,
-  output: ["html", "body.test-class", "article#i1-changed", "div#i11", "div#i111", "div#i1111", "b"]
+  output: ["html", "body.test-class", "article#i1-changed", "div#i11",
+           "div#i111", "div#i1111"]
 }, {
   desc: "Moving an undisplayed child in a displayed element should not refresh",
-  setup: function*() {},
-  run: function*({walker}) {
+  setup: function* () {},
+  run: function* ({walker}) {
     // Re-append #i2 in body (move it to the end).
     let parent = yield walker.querySelector(walker.rootNode, "body");
     let child = yield walker.querySelector(walker.rootNode, "#i2");
     yield walker.insertBefore(child, parent);
   },
   shouldRefresh: false,
-  output: ["html", "body.test-class", "article#i1-changed", "div#i11", "div#i111", "div#i1111", "b"]
+  output: ["html", "body.test-class", "article#i1-changed", "div#i11",
+           "div#i111", "div#i1111"]
 }, {
-  desc: "Updating attributes on an element that's not displayed should not refresh",
-  setup: function*() {},
-  run: function*({walker}) {
+  desc: "Updating attributes on an element that's not displayed should not " +
+        "refresh",
+  setup: function* () {},
+  run: function* ({walker}) {
     let node = yield walker.querySelector(walker.rootNode, "#i2");
     yield node.modifyAttributes([{
       attributeName: "id",
       newValue: "i2-changed"
     }, {
       attributeName: "class",
       newValue: "test-class"
     }]);
   },
   shouldRefresh: false,
-  output: ["html", "body.test-class", "article#i1-changed", "div#i11", "div#i111", "div#i1111", "b"]
+  output: ["html", "body.test-class", "article#i1-changed", "div#i11",
+           "div#i111", "div#i1111"]
 }, {
   desc: "Removing the currently selected node should refresh",
-  setup: function*(inspector) {
+  setup: function* (inspector) {
     yield selectNode("#i2-changed", inspector);
   },
-  run: function*({walker, selection}) {
+  run: function* ({walker, selection}) {
     yield walker.removeNode(selection.nodeFront);
   },
   shouldRefresh: true,
-  output: ["html", "body.test-class", "article#i1-changed"]
+  output: ["html", "body.test-class"]
 }, {
   desc: "Changing the class of the currently selected node should refresh",
-  setup: function*() {},
-  run: function*({selection}) {
+  setup: function* () {},
+  run: function* ({selection}) {
     yield selection.nodeFront.modifyAttributes([{
       attributeName: "class",
       newValue: "test-class-changed"
     }]);
   },
   shouldRefresh: true,
-  output: ["html", "body.test-class-changed", "article#i1-changed"]
+  output: ["html", "body.test-class-changed"]
 }, {
   desc: "Changing the id of the currently selected node should refresh",
-  setup: function*() {},
-  run: function*({selection}) {
+  setup: function* () {},
+  run: function* ({selection}) {
     yield selection.nodeFront.modifyAttributes([{
       attributeName: "id",
       newValue: "new-id"
     }]);
   },
   shouldRefresh: true,
-  output: ["html", "body#new-id.test-class-changed", "article#i1-changed"]
+  output: ["html", "body#new-id.test-class-changed"]
 }];
 
-add_task(function*() {
+add_task(function* () {
   let {inspector} = yield openInspectorForURL(TEST_URI);
   let container = inspector.panelDoc.getElementById("inspector-breadcrumbs");
   let win = container.ownerDocument.defaultView;
 
   for (let {desc, setup, run, shouldRefresh, output} of TEST_DATA) {
     info("Running test case: " + desc);
 
     info("Listen to markupmutation events from the inspector to know when a " +
@@ -190,14 +198,14 @@ add_task(function*() {
         "The breadcrumbs widget is not currently updating");
     }
 
     is(shouldRefresh, hasBreadcrumbsMutated, "Has the breadcrumbs refreshed?");
     observer.disconnect();
 
     info("Check the output of the breadcrumbs widget");
     is(container.childNodes.length, output.length, "Correct number of buttons");
-    for (let i = 0; i < container.childNodes.length; i ++) {
+    for (let i = 0; i < container.childNodes.length; i++) {
       is(output[i], container.childNodes[i].textContent,
         "Text content for button " + i + " is correct");
     }
   }
 });
--- a/devtools/client/inspector/test/browser_inspector_keyboard-shortcuts.js
+++ b/devtools/client/inspector/test/browser_inspector_keyboard-shortcuts.js
@@ -3,31 +3,37 @@
    http://creativecommons.org/publicdomain/zero/1.0/ */
 "use strict";
 
 // Tests that the keybindings for highlighting different elements work as
 // intended.
 
 const TEST_URI = "data:text/html;charset=utf-8," +
   "<html><head><title>Test for the highlighter keybindings</title></head>" +
-  "<body><h1>Hello</h1><p><strong>Greetings, earthlings!</strong>" +
+  "<body><p><strong>Greetings, earthlings!</strong>" +
   " I come in peace.</p></body></html>";
 
 const TEST_DATA = [
-  { key: "VK_RIGHT", selectedNode: "h1" },
-  { key: "VK_DOWN", selectedNode: "p" },
-  { key: "VK_UP", selectedNode: "h1" },
+  { key: "VK_LEFT", selectedNode: "p" },
   { key: "VK_LEFT", selectedNode: "body" },
+  { key: "VK_LEFT", selectedNode: "html" },
+  { key: "VK_RIGHT", selectedNode: "body" },
+  { key: "VK_RIGHT", selectedNode: "p" },
+  { key: "VK_RIGHT", selectedNode: "strong" },
 ];
 
 add_task(function* () {
   let { inspector } = yield openInspectorForURL(TEST_URI);
-  let bodyFront = yield getNodeFront("body", inspector);
-  is(inspector.selection.nodeFront, bodyFront,
-    "Body should be selected initially.");
+
+  info("Selecting the deepest element to start with");
+  yield selectNode("strong", inspector);
+
+  let nodeFront = yield getNodeFront("strong", inspector);
+  is(inspector.selection.nodeFront, nodeFront,
+     "<strong> should be selected initially");
 
   info("Focusing the currently active breadcrumb button");
   let bc = inspector.breadcrumbs;
   bc.nodeHierarchy[bc.currentIndex].button.focus();
 
   for (let { key, selectedNode } of TEST_DATA) {
     info("Pressing " + key + " to select " + selectedNode);
 
--- a/devtools/client/inspector/test/browser_inspector_search-06.js
+++ b/devtools/client/inspector/test/browser_inspector_search-06.js
@@ -15,34 +15,40 @@ add_task(function* () {
   info("Searching for test node #d1");
   yield focusSearchBoxUsingShortcut(inspector.panelWin);
   yield synthesizeKeys(["#", "d", "1", "VK_RETURN"], inspector);
 
   yield inspector.search.once("search-result");
   assertHasResult(inspector, true);
 
   info("Removing node #d1");
+  // Expect an inspector-updated event here, because removing #d1 causes the
+  // breadcrumbs to update (since #d1 is displayed in it).
+  let onUpdated = inspector.once("inspector-updated");
   yield mutatePage(inspector, testActor,
                    "document.getElementById(\"d1\").remove()");
+  yield onUpdated;
 
   info("Pressing return button to search again for node #d1.");
   yield synthesizeKeys("VK_RETURN", inspector);
 
   yield inspector.search.once("search-result");
   assertHasResult(inspector, false);
 
   info("Emptying the field and searching for a node that doesn't exist: #d3");
   let keys = ["VK_BACK_SPACE", "VK_BACK_SPACE", "VK_BACK_SPACE", "#", "d", "3",
               "VK_RETURN"];
   yield synthesizeKeys(keys, inspector);
 
   yield inspector.search.once("search-result");
   assertHasResult(inspector, false);
 
   info("Create the #d3 node in the page");
+  // No need to expect an inspector-updated event here, Creating #d3 isn't going
+  // to update the breadcrumbs in any ways.
   yield mutatePage(inspector, testActor,
                    `document.getElementById("d2").insertAdjacentHTML(
                     "afterend", "<div id=d3></div>")`);
 
   info("Pressing return button to search again for node #d3.");
   yield synthesizeKeys("VK_RETURN", inspector);
 
   yield inspector.search.once("search-result");
@@ -70,12 +76,12 @@ function* synthesizeKeys(keys, inspector
 
 function assertHasResult(inspector, expectResult) {
   is(inspector.searchBox.classList.contains("devtools-no-search-result"),
      !expectResult,
      "There are" + (expectResult ? "" : " no") + " search results");
 }
 
 function* mutatePage(inspector, testActor, expression) {
-  let onUpdated = inspector.once("inspector-updated");
+  let onMutation = inspector.once("markupmutation");
   yield testActor.eval(expression);
-  yield onUpdated;
+  yield onMutation;
 }