Bug 1107658 - fix undo for delete node when next sibling is a pseudo element;r=pbro draft
authorJulian Descottes <jdescottes@mozilla.com>
Thu, 02 Jun 2016 22:27:24 +0200
changeset 374796 548c109d9f7c02110474ecf622acd3c07194e20e
parent 374513 c2f28446c94a222c145c436d6d9e78ec68b7e17a
child 522684 d598bff073f35f66ef4a44549e29777e31a515b6
push id20078
push userjdescottes@mozilla.com
push dateThu, 02 Jun 2016 20:44:49 +0000
reviewerspbro
bugs1107658
milestone49.0a1
Bug 1107658 - fix undo for delete node when next sibling is a pseudo element;r=pbro Check that the stored sibling is a valid target for insertBefore, update existing test to cover this use case. MozReview-Commit-ID: 6yvLfSQMAJw
devtools/client/inspector/markup/markup.js
devtools/client/inspector/markup/test/browser_markup_tag_edit_04.js
--- a/devtools/client/inspector/markup/markup.js
+++ b/devtools/client/inspector/markup/markup.js
@@ -865,16 +865,18 @@ MarkupView.prototype = {
             focusNode = nextSibling || siblings.previousSibling || parent;
           }
 
           if (container.selected) {
             this.navigate(this.getContainer(focusNode));
           }
         });
       }, () => {
+        let isValidSibling = nextSibling && !nextSibling.isPseudoElement;
+        nextSibling = isValidSibling ? nextSibling : null;
         this.walker.insertBefore(node, parent, nextSibling);
       });
     }).then(null, console.error);
   },
 
   /**
    * If an editable item is focused, select its container.
    */
--- a/devtools/client/inspector/markup/test/browser_markup_tag_edit_04.js
+++ b/devtools/client/inspector/markup/test/browser_markup_tag_edit_04.js
@@ -3,101 +3,115 @@
  http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
 // Tests that a node can be deleted from the markup-view with the delete key.
 // Also checks that after deletion the correct element is highlighted.
 // The next sibling is preferred, but the parent is a fallback.
 
-const HTML = `<div id="parent">
+const HTML = `<style type="text/css">
+                #pseudo::before { content: 'before'; }
+                #pseudo::after { content: 'after'; }
+              </style>
+              <div id="parent">
                 <div id="first"></div>
                 <div id="second"></div>
                 <div id="third"></div>
+              </div>
+              <div id="only-child">
+                <div id="fourth"></div>
+              </div>
+              <div id="pseudo">
+                <div id="fifth"></div>
               </div>`;
 const TEST_URL = "data:text/html;charset=utf-8," + encodeURIComponent(HTML);
 
 // List of all the test cases. Each item is an object with the following props:
 // - selector: the css selector of the node that should be selected
 // - key: the key to press to delete the node (delete or back_space)
 // - focusedSelector: the css selector of the node we expect to be selected as
 //   a result of the deletion
-// - setup: an optional function that will be run before selecting and deleting
-//   the node
+// - pseudo: (optional) if the focused node is actually supposed to be a pseudo element
+//   of the specified selector.
 // Note that after each test case, undo is called.
 const TEST_DATA = [{
   selector: "#first",
   key: "delete",
   focusedSelector: "#second"
 }, {
   selector: "#second",
   key: "delete",
   focusedSelector: "#third"
 }, {
   selector: "#third",
   key: "delete",
   focusedSelector: "#second"
 }, {
+  selector: "#fourth",
+  key: "delete",
+  focusedSelector: "#only-child"
+}, {
+  selector: "#fifth",
+  key: "delete",
+  focusedSelector: "#pseudo",
+  pseudo: "after"
+}, {
   selector: "#first",
   key: "back_space",
   focusedSelector: "#second"
 }, {
   selector: "#second",
   key: "back_space",
   focusedSelector: "#first"
 }, {
   selector: "#third",
   key: "back_space",
   focusedSelector: "#second"
 }, {
-  setup: function* (inspector, testActor) {
-    // Removing the siblings of #first in order to test with an only child.
-    let mutated = inspector.once("markupmutation");
-    yield testActor.eval(`
-      for (let node of content.document.querySelectorAll("#second, #third")) {
-        node.remove();
-      }
-    `);
-    yield mutated;
-  },
-  selector: "#first",
-  key: "delete",
-  focusedSelector: "#parent"
+  selector: "#fourth",
+  key: "back_space",
+  focusedSelector: "#only-child"
 }, {
-  selector: "#first",
+  selector: "#fifth",
   key: "back_space",
-  focusedSelector: "#parent"
+  focusedSelector: "#pseudo",
+  pseudo: "before"
 }];
 
 add_task(function* () {
-  let {inspector, testActor} = yield openInspectorForURL(TEST_URL);
+  let {inspector} = yield openInspectorForURL(TEST_URL);
 
-  for (let {setup, selector, key, focusedSelector} of TEST_DATA) {
-    if (setup) {
-      yield setup(inspector, testActor);
-    }
-
-    yield checkDeleteAndSelection(inspector, key, selector, focusedSelector);
+  for (let data of TEST_DATA) {
+    yield checkDeleteAndSelection(inspector, data);
   }
 });
 
-function* checkDeleteAndSelection(inspector, key, selector, focusedSelector) {
+function* checkDeleteAndSelection(inspector, {key, selector, focusedSelector, pseudo}) {
   info("Test deleting node " + selector + " with " + key + ", " +
        "expecting " + focusedSelector + " to be focused");
 
   info("Select node " + selector + " and make sure it is focused");
   yield selectNode(selector, inspector);
   yield clickContainer(selector, inspector);
 
   info("Delete the node with: " + key);
   let mutated = inspector.once("markupmutation");
   EventUtils.sendKey(key, inspector.panelWin);
   yield Promise.all([mutated, inspector.once("inspector-updated")]);
 
   let nodeFront = yield getNodeFront(focusedSelector, inspector);
+  if (pseudo) {
+    // Update the selector for logging in case of failure.
+    focusedSelector = focusedSelector + "::" + pseudo;
+    // Retrieve the :before or :after pseudo element of the nodeFront.
+    let {nodes} = yield inspector.walker.children(nodeFront);
+    nodeFront = pseudo === "before" ? nodes[0] : nodes[nodes.length - 1];
+  }
+
   is(inspector.selection.nodeFront, nodeFront,
      focusedSelector + " is selected after deletion");
 
   info("Check that the node was really removed");
   let node = yield getNodeFront(selector, inspector);
   ok(!node, "The node can't be found in the page anymore");
 
   info("Undo the deletion to restore the original markup");