Bug 1262491 - Focus the newly inserted node in the markup-view when clicking the add button. r=jdescottes
authorPatrick Brosset <pbrosset@mozilla.com>
Thu, 14 Apr 2016 13:06:41 +0200
changeset 333323 2013ce74d60df378be9a0d5989533d3f9460cf56
parent 333322 735739a3a13aff3b07b0b839b070b6a4e8af9914
child 333324 bdc6a9d57ec4a4622848235c7356da4d3008f21e
push id1146
push userCallek@gmail.com
push dateMon, 25 Jul 2016 16:35:44 +0000
treeherdermozilla-release@a55778f9cd5a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdescottes
bugs1262491
milestone48.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 1262491 - Focus the newly inserted node in the markup-view when clicking the add button. r=jdescottes MozReview-Commit-ID: FOyFBXT20Du
devtools/client/inspector/breadcrumbs.js
devtools/client/inspector/inspector-panel.js
devtools/client/inspector/markup/markup.js
devtools/client/inspector/test/browser_inspector_addNode_03.js
--- a/devtools/client/inspector/breadcrumbs.js
+++ b/devtools/client/inspector/breadcrumbs.js
@@ -122,19 +122,20 @@ HTMLBreadcrumbs.prototype = {
     };
   },
 
   /**
    * Warn if rejection was caused by selection change, print an error otherwise.
    * @param {Error} err
    */
   selectionGuardEnd: function(err) {
-    if (err === "selection-changed") {
-      console.warn("Asynchronous operation was aborted as selection changed.");
-    } else {
+    // 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}
@@ -613,17 +614,17 @@ HTMLBreadcrumbs.prototype = {
           }
           lastNode = node;
         }
         if (response.hasLast) {
           deferred.resolve(fallback);
           return;
         }
         moreChildren();
-      }).then(null, this.selectionGuardEnd);
+      }).catch(this.selectionGuardEnd);
     };
 
     moreChildren();
     return deferred.promise;
   },
 
   /**
    * Find the "youngest" ancestor of a node which is already in the breadcrumbs.
@@ -811,17 +812,17 @@ HTMLBreadcrumbs.prototype = {
       this.updateSelectors();
 
       // Make sure the selected node and its neighbours are visible.
       this.scroll();
       return resolveNextTick().then(() => {
         this.inspector.emit("breadcrumbs-updated", this.selection.nodeFront);
         doneUpdating();
       });
-    }).then(null, err => {
+    }).catch(err => {
       doneUpdating(this.selection.nodeFront);
       this.selectionGuardEnd(err);
     });
   }
 };
 
 /**
  * Returns a promise that resolves at the next main thread tick.
--- a/devtools/client/inspector/inspector-panel.js
+++ b/devtools/client/inspector/inspector-panel.js
@@ -1061,17 +1061,17 @@ InspectorPanel.prototype = {
 
     // Insert the html and expect a childList markup mutation.
     let onMutations = this.once("markupmutation");
     let {nodes} = yield this.walker.insertAdjacentHTML(this.selection.nodeFront,
                                                        "beforeEnd", html);
     yield onMutations;
 
     // Select the new node (this will auto-expand its parent).
-    this.selection.setNodeFront(nodes[0]);
+    this.selection.setNodeFront(nodes[0], "node-inserted");
   }),
 
   /**
    * Toggle a pseudo class.
    */
   togglePseudoClass: function(aPseudo) {
     if (this.selection.isElementNode()) {
       let node = this.selection.nodeFront;
--- a/devtools/client/inspector/markup/markup.js
+++ b/devtools/client/inspector/markup/markup.js
@@ -545,29 +545,35 @@ MarkupView.prototype = {
           "destroyed while showing the node.");
       }
     });
 
     promise.all([onShowBoxModel, onShow]).then(done);
   },
 
   /**
-   * Focus the current node selection's MarkupContainer if the selection
-   * happened because the user picked an element using the element picker or
-   * browser context menu.
+   * Maybe focus the current node selection's MarkupContainer depending on why
+   * the current node got selected.
    */
   maybeFocusNewSelection: function() {
     let {reason, nodeFront} = this._inspector.selection;
 
-    if (reason !== "browser-context-menu" &&
-        reason !== "picker-node-picked") {
-      return;
+    // The list of reasons that should lead to focusing the node.
+    let reasonsToFocus = [
+      // If the user picked an element with the element picker.
+      "picker-node-picked",
+      // If the user selected an element with the browser context menu.
+      "browser-context-menu",
+      // If the user added a new node by clicking in the inspector toolbar.
+      "node-inserted"
+    ];
+
+    if (reasonsToFocus.includes(reason)) {
+      this.getContainer(nodeFront).focus();
     }
-
-    this.getContainer(nodeFront).focus();
   },
 
   /**
    * Create a TreeWalker to find the next/previous
    * node for selection.
    */
   _selectionWalker: function(start) {
     let walker = this.doc.createTreeWalker(
--- a/devtools/client/inspector/test/browser_inspector_addNode_03.js
+++ b/devtools/client/inspector/test/browser_inspector_addNode_03.js
@@ -1,20 +1,20 @@
 /* vim: set 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 adding nodes does work as expected: the parent gets expanded and
-// the new node gets selected.
+// Test that adding nodes does work as expected: the parent gets expanded, the
+// new node gets selected and the corresponding markup-container focused.
 
 const TEST_URL = URL_ROOT + "doc_inspector_add_node.html";
 
-add_task(function*() {
+add_task(function* () {
   let {inspector} = yield openInspectorForURL(TEST_URL);
 
   info("Adding in element that has no children and is collapsed");
   let parentNode = yield getNodeFront("#foo", inspector);
   yield selectNode(parentNode, inspector);
   yield testAddNode(parentNode, inspector);
 
   info("Adding in element with children but that has not been expanded yet");
@@ -32,37 +32,45 @@ add_task(function*() {
   info("Adding in element with children that is expanded");
   parentNode = yield getNodeFront("#bar", inspector);
   yield selectNode(parentNode, inspector);
   yield testAddNode(parentNode, inspector);
 });
 
 function* testAddNode(parentNode, inspector) {
   let btn = inspector.panelDoc.querySelector("#inspector-element-add-button");
+  let markupWindow = inspector.markup.win;
 
-  info("Clicking on the 'add node' button and expecting a markup mutation");
+  info("Clicking 'add node' and expecting a markup mutation and focus event");
   let onMutation = inspector.once("markupmutation");
   btn.click();
   let mutations = yield onMutation;
 
-  info("Expecting an inspector-updated event right after the mutation event "+
+  info("Expecting an inspector-updated event right after the mutation event " +
        "to wait for the new node selection");
   yield inspector.once("inspector-updated");
 
   is(mutations.length, 1, "There is one mutation only");
   is(mutations[0].added.length, 1, "There is one new node only");
 
   let newNode = mutations[0].added[0];
 
   is(newNode, inspector.selection.nodeFront,
      "The new node is selected");
 
   ok(inspector.markup.getContainer(parentNode).expanded,
      "The parent node is now expanded");
 
   is(inspector.selection.nodeFront.parentNode(), parentNode,
      "The new node is inside the right parent");
+
+  let focusedElement = markupWindow.document.activeElement;
+  let focusedContainer = focusedElement.closest(".child").container;
+  is(focusedContainer.node, inspector.selection.nodeFront,
+     "The right container is focused in the markup-view");
+  ok(focusedElement.classList.contains("tag"),
+     "The tagName part of the container is focused");
 }
 
 function collapseNode(node, inspector) {
   let container = inspector.markup.getContainer(node);
   container.setExpanded(false);
 }