Bug 1262491 - Focus the newly inserted node in the markup-view when clicking the add button; r=jdescottes draft
authorPatrick Brosset <pbrosset@mozilla.com>
Thu, 14 Apr 2016 13:06:41 +0200
changeset 351973 bdd73d0bc1683bbc95d066c410a047b4e4df9959
parent 351972 529cff9c5663a3581dfb5e690727144f07e8e23c
child 518538 7b83efd738ff9d81fc361de81f5dbe6ac15e5e95
push id15569
push userpbrosset@mozilla.com
push dateFri, 15 Apr 2016 10:09:54 +0000
reviewersjdescottes
bugs1262491
milestone48.0a1
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);
 }