Bug 1269226 - Explicitly handle node deletion in inspector breadcrumbs. r=pbro
☠☠ backed out by 2afb6c749a08 ☠ ☠
authorSteve Melia <steve.j.melia@gmail.com>
Wed, 25 May 2016 00:06:28 +0100
changeset 338097 9cfc40ea2dbd0e258ec58028d85f4b358162e64a
parent 338096 f5ce758ba0fdf72adcc8637be3206d524872040e
child 338098 5fdfce654062b471415a82f1a2c07200fc463dcf
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)
reviewerspbro
bugs1269226
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 1269226 - Explicitly handle node deletion in inspector breadcrumbs. r=pbro
devtools/client/inspector/breadcrumbs.js
devtools/client/inspector/markup/markup.js
devtools/client/inspector/test/browser_inspector_delete-selected-node-02.js
devtools/client/inspector/test/doc_inspector_delete-selected-node-02.html
--- a/devtools/client/inspector/breadcrumbs.js
+++ b/devtools/client/inspector/breadcrumbs.js
@@ -579,16 +579,32 @@ HTMLBreadcrumbs.prototype = {
                      cmdDispatcher.focusedElement.parentNode == this.container);
 
     if (!this.selection.isConnected()) {
       // remove all the crumbs
       this.cutAfter(-1);
       return;
     }
 
+    // If this was an interesting deletion; then trim the breadcrumb trail
+    if (reason === "markupmutation") {
+      for (let {type, removed} of mutations) {
+        if (type !== "childList") {
+          continue;
+        }
+
+        for (let node of removed) {
+          let removedIndex = this.indexOf(node);
+          if (removedIndex > -1) {
+            this.cutAfter(removedIndex - 1);
+          }
+        }
+      }
+    }
+
     if (!this.selection.isElementNode()) {
       // no selection
       this.setCursor(-1);
       return;
     }
 
     let idx = this.indexOf(this.selection.nodeFront);
 
--- a/devtools/client/inspector/markup/markup.js
+++ b/devtools/client/inspector/markup/markup.js
@@ -852,22 +852,36 @@ MarkupView.prototype = {
 
     // Retain the node so we can undo this...
     this.walker.retainNode(node).then(() => {
       let parent = node.parentNode();
       let nextSibling = null;
       this.undo.do(() => {
         this.walker.removeNode(node).then(siblings => {
           nextSibling = siblings.nextSibling;
-          let focusNode = moveBackward ? siblings.previousSibling : nextSibling;
+          let prevSibling = siblings.previousSibling;
+          let focusNode = moveBackward ? prevSibling : nextSibling;
 
           // If we can't move as the user wants, we move to the other direction.
           // If there is no sibling elements anymore, move to the parent node.
           if (!focusNode) {
-            focusNode = nextSibling || siblings.previousSibling || parent;
+            focusNode = nextSibling || prevSibling || parent;
+          }
+
+          let isNextSiblingText = nextSibling ?
+            nextSibling.nodeType === Ci.nsIDOMNode.TEXT_NODE : false;
+          let isPrevSiblingText = prevSibling ?
+            prevSibling.nodeType === Ci.nsIDOMNode.TEXT_NODE : false;
+
+          // If the parent had two children and the next or previous sibling
+          // is a text node, then it now has only a single text node, is about
+          // to be in-lined; and focus should move to the parent.
+          if (parent.numChildren === 2
+              && (isNextSiblingText || isPrevSiblingText)) {
+            focusNode = parent;
           }
 
           if (container.selected) {
             this.navigate(this.getContainer(focusNode));
           }
         });
       }, () => {
         this.walker.insertBefore(node, parent, nextSibling);
--- a/devtools/client/inspector/test/browser_inspector_delete-selected-node-02.js
+++ b/devtools/client/inspector/test/browser_inspector_delete-selected-node-02.js
@@ -13,41 +13,25 @@ const TEST_PAGE = URL_ROOT +
   "doc_inspector_delete-selected-node-02.html";
 
 add_task(function* () {
   let { inspector } = yield openInspectorForURL(TEST_PAGE);
 
   yield testManuallyDeleteSelectedNode();
   yield testAutomaticallyDeleteSelectedNode();
   yield testDeleteSelectedNodeContainerFrame();
+  yield testDeleteWithNonElementNode();
 
   function* testManuallyDeleteSelectedNode() {
     info("Selecting a node, deleting it via context menu and checking that " +
-          "its parent node is selected and breadcrumbs are updated.");
-
-    yield selectNode("#deleteManually", inspector);
-
-    info("Getting the node container in the markup view.");
-    let container = yield getContainerForSelector("#deleteManually", inspector);
-
-    info("Simulating right-click on the markup view container.");
-    EventUtils.synthesizeMouse(container.tagLine, 2, 2,
-      {type: "contextmenu", button: 2}, inspector.panelWin);
+         "its parent node is selected and breadcrumbs are updated.");
 
-    info("Waiting for the context menu to open.");
-    yield once(inspector.panelDoc.getElementById("inspectorPopupSet"),
-               "popupshown");
+    yield deleteNodeWithContextMenu("#deleteManually");
 
-    info("Clicking 'Delete Node' in the context menu.");
-    inspector.panelDoc.getElementById("node-menu-delete").click();
-
-    info("Waiting for inspector to update.");
-    yield inspector.once("inspector-updated");
-
-    info("Inspector updated, performing checks.");
+    info("Performing checks.");
     yield assertNodeSelectedAndPanelsUpdated("#selectedAfterDelete",
                                              "li#selectedAfterDelete");
   }
 
   function* testAutomaticallyDeleteSelectedNode() {
     info("Selecting a node, deleting it via javascript and checking that " +
          "its parent node is selected and breadcrumbs are updated.");
 
@@ -80,19 +64,87 @@ add_task(function* () {
 
     info("Waiting for inspector to update.");
     yield inspector.once("inspector-updated");
 
     info("Inspector updated, performing checks.");
     yield assertNodeSelectedAndPanelsUpdated("body", "body");
   }
 
+  function* testDeleteWithNonElementNode() {
+    info("Selecting a node, deleting it via context menu and checking that " +
+         "its parent node is selected and breadcrumbs are updated " +
+         "when the node is followed by a non-element node");
+
+    yield deleteNodeWithContextMenu("#deleteWithNonElement");
+
+    let expectedCrumbs = ["html", "body", "div#deleteToMakeSingleTextNode"];
+    yield assertNodeSelectedAndCrumbsUpdated(expectedCrumbs,
+                                             Node.TEXT_NODE);
+
+    let div = yield getNodeFront("#deleteToMakeSingleTextNode", inspector);
+    let {nodes} = yield inspector.walker.children(div);
+    let secondTextNode = nodes[1];
+    yield deleteNodeWithKey(secondTextNode);
+    expectedCrumbs = ["html", "body", "div#deleteToMakeSingleTextNode"];
+    yield assertNodeSelectedAndCrumbsUpdated(expectedCrumbs,
+                                             Node.ELEMENT_NODE);
+  }
+
+  function* deleteNodeWithKey(selector) {
+    yield selectNode(selector, inspector);
+
+    info("Simulating delete keypress on the markup view container.");
+    EventUtils.synthesizeKey("VK_DELETE", {}, inspector.panelWin);
+
+    info("Waiting for inspector to update.");
+    yield inspector.once("inspector-updated");
+  }
+
+  function* deleteNodeWithContextMenu(selector) {
+    yield selectNode(selector, inspector);
+
+    info("Getting the node container in the markup view.");
+    let container = yield getContainerForSelector(selector, inspector);
+
+    info("Simulating right-click on the markup view container.");
+    EventUtils.synthesizeMouse(container.tagLine, 2, 2,
+      {type: "contextmenu", button: 2}, inspector.panelWin);
+
+    info("Waiting for the context menu to open.");
+    let popupSet = inspector.panelDoc.getElementById("inspectorPopupSet");
+    yield once(popupSet, "popupshown");
+
+    info("Clicking 'Delete Node' in the context menu.");
+    inspector.panelDoc.getElementById("node-menu-delete").click();
+
+    info("Waiting for inspector to update.");
+    yield inspector.once("inspector-updated");
+  }
+
+  function* assertNodeSelectedAndCrumbsUpdated(expectedCrumbs,
+                                               expectedNodeType) {
+    info("Performing checks");
+    let actualNodeType = inspector.selection.nodeFront.nodeType;
+    is(actualNodeType, expectedNodeType, "The node has the right type");
+
+    let breadcrumbs = inspector.panelDoc
+                               .getElementById("inspector-breadcrumbs")
+                               .childNodes;
+    is(breadcrumbs.length, expectedCrumbs.length,
+       "Have the correct number of breadcrumbs");
+    for (let i = 0; i < breadcrumbs.length; i++) {
+      is(breadcrumbs[i].textContent, expectedCrumbs[i],
+         "Text content for button " + i + " is correct");
+    }
+  }
+
   function* assertNodeSelectedAndPanelsUpdated(selector, crumbLabel) {
     let nodeFront = yield getNodeFront(selector, inspector);
     is(inspector.selection.nodeFront, nodeFront, "The right node is selected");
 
-    let breadcrumbs = inspector.panelDoc.getElementById(
-      "inspector-breadcrumbs");
+    let breadcrumbs = inspector.panelDoc
+                               .getElementById("inspector-breadcrumbs");
     is(breadcrumbs.querySelector("button[checked=true]").textContent,
        crumbLabel,
        "The right breadcrumb is selected");
   }
 });
--- a/devtools/client/inspector/test/doc_inspector_delete-selected-node-02.html
+++ b/devtools/client/inspector/test/doc_inspector_delete-selected-node-02.html
@@ -6,10 +6,15 @@
 </head>
 <body>
   <ul id="deleteChildren">
     <li id="deleteManually">Delete me via the inspector</li>
     <li id="selectedAfterDelete">This node is selected after manual delete</li>
     <li id="deleteAutomatically">Delete me via javascript</li>
   </ul>
   <iframe id="deleteIframe" src="data:text/html,%3C!DOCTYPE%20html%3E%3Chtml%20lang%3D%22en%22%3E%3Cbody%3E%3Cp%20id%3D%22deleteInIframe%22%3EDelete my container iframe%3C%2Fp%3E%3C%2Fbody%3E%3C%2Fhtml%3E"></iframe>
+  <div id="deleteToMakeSingleTextNode">
+    1
+    <b id="deleteWithNonElement">Delete me and select the non-element node</b>
+    2
+  </div>
 </body>
 </html>