Bug 1355886 - fix regression when undoing delete node next to a whitespace sibling;r=pbro
MozReview-Commit-ID: GDYzsbCiQcI
--- a/devtools/client/inspector/markup/test/browser_markup_tag_delete_whitespace_node.js
+++ b/devtools/client/inspector/markup/test/browser_markup_tag_delete_whitespace_node.js
@@ -6,36 +6,30 @@
// After deleting a node, whitespace siblings that had an impact on the layout might no
// longer have any impact. This tests that the markup view is correctly rendered after
// deleting a node that triggers such a change.
const HTML =
`<div>
<p id="container">
- <span>1</span> <span id="after-whitespace">2</span>
+ <span id="before-whitespace">1</span> <span id="after-whitespace">2</span>
</p>
</div>`;
const TEST_URL = "data:text/html;charset=utf-8," + encodeURIComponent(HTML);
-add_task(function* () {
+add_task(function* deleteNodeAfterWhitespace() {
let {inspector} = yield openInspectorForURL(TEST_URL);
info("Test deleting a node that will modify the whitespace nodes rendered in the " +
"markup view.");
- info("Select node #after-whitespace and make sure it is focused");
- yield selectNode("#after-whitespace", inspector);
- yield clickContainer("#after-whitespace", inspector);
-
- info("Delete the node with the delete key");
- let mutated = inspector.once("markupmutation");
- EventUtils.sendKey("delete", inspector.panelWin);
- yield Promise.all([mutated, inspector.once("inspector-updated")]);
+ yield selectAndFocusNode("#after-whitespace", inspector);
+ yield deleteCurrentSelection(inspector);
// TODO: There is still an issue with selection here. When the span is deleted, the
// selection goes to text-node. But since the text-node gets removed from the markup
// view after losing its impact on the layout, the selection remains on a node which
// is no longer part of the markup view (but still a valid node in the content DOM).
let parentNodeFront = yield inspector.selection.nodeFront.parentNode();
let nodeFront = yield getNodeFront("#container", inspector);
is(parentNodeFront, nodeFront, "Selection is as expected after deletion");
@@ -43,9 +37,37 @@ add_task(function* () {
info("Check that the node was really removed");
let node = yield getNodeFront("#after-whitespace", inspector);
ok(!node, "The node can't be found in the page anymore");
info("Undo the deletion to restore the original markup");
yield undoChange(inspector);
node = yield getNodeFront("#after-whitespace", inspector);
ok(node, "The node is back");
+
+ info("Test deleting the node before the whitespace and performing an undo preserves " +
+ "the node order");
+
+ yield selectAndFocusNode("#before-whitespace", inspector);
+ yield deleteCurrentSelection(inspector);
+
+ info("Undo the deletion to restore the original markup");
+ yield undoChange(inspector);
+ node = yield getNodeFront("#before-whitespace", inspector);
+ ok(node, "The node is back");
+
+ let nextSibling = yield getNodeFront("#before-whitespace + *", inspector);
+ let afterWhitespace = yield getNodeFront("#after-whitespace", inspector);
+ is(nextSibling, afterWhitespace, "Order has been preserved after restoring the node");
});
+
+function* selectAndFocusNode(selector, inspector) {
+ info(`Select node ${selector} and make sure it is focused`);
+ yield selectNode(selector, inspector);
+ yield clickContainer(selector, inspector);
+}
+
+function* deleteCurrentSelection(inspector) {
+ info("Delete the node with the delete key");
+ let mutated = inspector.once("markupmutation");
+ EventUtils.sendKey("delete", inspector.panelWin);
+ yield Promise.all([mutated, inspector.once("inspector-updated")]);
+}
--- a/devtools/server/actors/inspector.js
+++ b/devtools/server/actors/inspector.js
@@ -3118,22 +3118,22 @@ DocumentWalker.prototype = {
// Loop on starting node siblings.
let previous = node;
let next = node;
while (previous || next) {
previous = previous && previous.previousSibling;
next = next && next.nextSibling;
- if (this.filter(previous) === nodeFilterConstants.FILTER_ACCEPT) {
+ if (previous && this.filter(previous) === nodeFilterConstants.FILTER_ACCEPT) {
// A valid node was found in the previous siblings of the node.
return previous;
}
- if (this.filter(next) === nodeFilterConstants.FILTER_ACCEPT) {
+ if (next && this.filter(next) === nodeFilterConstants.FILTER_ACCEPT) {
// A valid node was found in the next siblings of the node.
return next;
}
}
return null;
},