Bug 1453872 - Make HTMLEditRules::JoinNodesSmart() return { aRightNode - aLeftNode.Length() } by default. r=m_kato, a=jcristau
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 13 Apr 2018 13:18:13 +0900
changeset 463296 01e680ea0270b9b84e409c686541b0db4dff3c54
parent 463295 9718d92fab4d9a39acdc2afb0302b6fcd7997f6c
child 463297 a8a5ba53d104721c7c586c53bc52a61d82a53d73
push id1683
push usersfraser@mozilla.com
push dateThu, 26 Apr 2018 16:43:40 +0000
treeherdermozilla-release@5af6cb21869d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato, jcristau
bugs1453872, 1423835
milestone60.0
Bug 1453872 - Make HTMLEditRules::JoinNodesSmart() return { aRightNode - aLeftNode.Length() } by default. r=m_kato, a=jcristau This is regression of bug 1423835. When I fixed the bug, I accidentally changed the result of HTMLEditRules::JoinNodesSmart() to use new API. However, it was simple misunderstand. The original code sets the initial value of result to { aRightNode - aLeftNode.Length() } but I understood it as { aRightNode - aRightNode.Length() }. Therefore, this patch backs out the patch only for this line. MozReview-Commit-ID: 5rD7YFij8v
editor/libeditor/HTMLEditRules.cpp
testing/web-platform/mozilla/meta/MANIFEST.json
testing/web-platform/mozilla/meta/editor/joining_nodes.html.ini
testing/web-platform/mozilla/tests/editor/joining_nodes.html
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -7742,18 +7742,17 @@ HTMLEditRules::JoinNodesSmart(nsIContent
       return EditorDOMPoint();
     }
     nsresult rv = mHTMLEditor->MoveNode(&aNodeRight, parent, parOffset);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return EditorDOMPoint();
     }
   }
 
-  EditorDOMPoint ret;
-  ret.SetToEndOf(&aNodeRight);
+  EditorDOMPoint ret(&aNodeRight, aNodeLeft.Length());
 
   // Separate join rules for differing blocks
   if (HTMLEditUtils::IsList(&aNodeLeft) || aNodeLeft.GetAsText()) {
     // For lists, merge shallow (wouldn't want to combine list items)
     nsresult rv = mHTMLEditor->JoinNodes(aNodeLeft, aNodeRight);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return EditorDOMPoint();
     }
--- a/testing/web-platform/mozilla/meta/MANIFEST.json
+++ b/testing/web-platform/mozilla/meta/MANIFEST.json
@@ -470,16 +470,22 @@
     ]
    ],
    "editor/initial_selection_on_focus.html": [
     [
      "/_mozilla/editor/initial_selection_on_focus.html",
      {}
     ]
    ],
+   "editor/joining_nodes.html": [
+    [
+     "/_mozilla/editor/joining_nodes.html",
+     {}
+    ]
+   ],
    "fetch/api/redirect/redirect-referrer.https.html": [
     [
      "/_mozilla/fetch/api/redirect/redirect-referrer.https.html",
      {}
     ]
    ],
    "focus/Range_collapse.html": [
     [
@@ -1027,16 +1033,20 @@
   "dom/throttling/throttling-ws.window.js": [
    "67a981ba2a4d08b684947ed42aba6648dcd262b4",
    "testharness"
   ],
   "editor/initial_selection_on_focus.html": [
    "06948dbf72160a7de5a0baaa2f6cf1bb54fbeb8f",
    "testharness"
   ],
+  "editor/joining_nodes.html": [
+   "048cf7d99acdecb927f97c4554c4d04ca8b15a8a",
+   "testharness"
+  ],
   "fetch/api/redirect/redirect-referrer-mixed-content.js": [
    "f9d7ec9cf9fa8c847e45664b05482e3f8c191385",
    "support"
   ],
   "fetch/api/redirect/redirect-referrer.https.html": [
    "99cbd16b78771f35e075e4012d8dbc5dce3209c0",
    "testharness"
   ],
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/mozilla/meta/editor/joining_nodes.html.ini
@@ -0,0 +1,86 @@
+[joining_nodes.html]
+  type: testharness
+  [Joining <dt> and <dd> nodes, delete command]
+    expected: FAIL
+
+  [Joining <dt> and <dd> nodes, forwarddelete command]
+    expected: FAIL
+
+  [Joining <dd> and <dt> nodes, delete command]
+    expected: FAIL
+
+  [Joining <dd> and <dt> nodes, forwarddelete command]
+    expected: FAIL
+
+  [Joining <h1> and <p> elements, delete command]
+    expected: FAIL
+
+  [Joining <h1> and <p> elements, forwarddelete command]
+    expected: FAIL
+
+  [Joining <h2> and <p> elements, delete command]
+    expected: FAIL
+
+  [Joining <h2> and <p> elements, forwarddelete command]
+    expected: FAIL
+
+  [Joining <h3> and <p> elements, delete command]
+    expected: FAIL
+
+  [Joining <h3> and <p> elements, forwarddelete command]
+    expected: FAIL
+
+  [Joining <h4> and <p> elements, delete command]
+    expected: FAIL
+
+  [Joining <h4> and <p> elements, forwarddelete command]
+    expected: FAIL
+
+  [Joining <h5> and <p> elements, delete command]
+    expected: FAIL
+
+  [Joining <h5> and <p> elements, forwarddelete command]
+    expected: FAIL
+
+  [Joining <h6> and <p> elements, delete command]
+    expected: FAIL
+
+  [Joining <h6> and <p> elements, forwarddelete command]
+    expected: FAIL
+
+  [Joining <p> and <h1> elements, delete command]
+    expected: FAIL
+
+  [Joining <p> and <h1> elements, forwarddelete command]
+    expected: FAIL
+
+  [Joining <p> and <h2> elements, delete command]
+    expected: FAIL
+
+  [Joining <p> and <h2> elements, forwarddelete command]
+    expected: FAIL
+
+  [Joining <p> and <h3> elements, delete command]
+    expected: FAIL
+
+  [Joining <p> and <h3> elements, forwarddelete command]
+    expected: FAIL
+
+  [Joining <p> and <h4> elements, delete command]
+    expected: FAIL
+
+  [Joining <p> and <h4> elements, forwarddelete command]
+    expected: FAIL
+
+  [Joining <p> and <h5> elements, delete command]
+    expected: FAIL
+
+  [Joining <p> and <h5> elements, forwarddelete command]
+    expected: FAIL
+
+  [Joining <p> and <h6> elements, delete command]
+    expected: FAIL
+
+  [Joining <p> and <h6> elements, forwarddelete command]
+    expected: FAIL
+
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/mozilla/tests/editor/joining_nodes.html
@@ -0,0 +1,256 @@
+<!doctype html>
+<html>
+<head>
+<meta charset=utf-8>
+<title>Joining nodes with delete/forwardDelete command</title>
+<script src=/resources/testharness.js></script>
+<script src=/resources/testharnessreport.js></script>
+</head>
+<body>
+<script>
+"use strict";
+
+(function() {
+  const kTests = [
+    { description: "Joining text nodes separated by <br>",
+      innerHTML: "<p>foo bar<br id=\"separator\">baz</p>",
+      expectedInnerHTML: "<p>foo barbaz</p>",
+      expectedSelectionRange: function (editor) {
+        return { collapsed: true,
+                 startContainer: editor.firstChild.firstChild,
+                 startOffset: 7 };
+      }, },
+    // XXX Attributes of right <li> element are cloned but this may not be expected behavior.
+    { description: "Joining <li> nodes in <ul>",
+      innerHTML: "<ul><li>foo bar</li><li id=\"separator\">baz</li></ul>",
+      expectedInnerHTML: "<ul><li id=\"separator\">foo barbaz</li></ul>",
+      expectedSelectionRange: function (editor) {
+        return { collapsed: true,
+                 startContainer: editor.firstChild.firstChild.firstChild,
+                 startOffset: 7 };
+      }, },
+    // XXX Attributes of right <li> element are cloned but this may not be expected behavior.
+    { description: "Joining <li> nodes in <ol>",
+      innerHTML: "<ol><li>foo bar</li><li id=\"separator\">baz</li></ol>",
+      expectedInnerHTML: "<ol><li id=\"separator\">foo barbaz</li></ol>",
+      expectedSelectionRange: function (editor) {
+        return { collapsed: true,
+                 startContainer: editor.firstChild.firstChild.firstChild,
+                 startOffset: 7 };
+      }, },
+    { description: "Joining <dt> and <dd> nodes",
+      innerHTML: "<dl><dt>foo bar</dt><dd id=\"separator\">baz</dd></dl>",
+      expectedInnerHTML: "<dl><dt>foo barbaz</dt></dl>",
+      expectedSelectionRange: function (editor) {
+        return { collapsed: true,
+                 startContainer: editor.firstChild.firstChild.firstChild,
+                 startOffset: 7 };
+      }, },
+    { description: "Joining <dd> and <dt> nodes",
+      innerHTML: "<dl><dd>foo bar</dd><dt id=\"separator\">baz</dt></dl>",
+      expectedInnerHTML: "<dl><dd>foo barbaz</dd></dl>",
+      expectedSelectionRange: function (editor) {
+        return { collapsed: true,
+                 startContainer: editor.firstChild.firstChild.firstChild,
+                 startOffset: 7 };
+      }, },
+    // XXX Attributes of right <p> element are cloned but this may not be expected behavior.
+    { description: "Joining <p> elements",
+      innerHTML: "<p>foo bar</p><p id=\"separator\">baz</p>",
+      expectedInnerHTML: "<p id=\"separator\">foo barbaz</p>",
+      expectedSelectionRange: function (editor) {
+        return { collapsed: true,
+                 startContainer: editor.firstChild.firstChild,
+                 startOffset: 7 };
+      }, },
+    // XXX Attributes of right <div> element are cloned but this may not be expected behavior.
+    { description: "Joining <div> elements",
+      innerHTML: "<div>foo bar</div><div id=\"separator\">baz</div>",
+      expectedInnerHTML: "<div id=\"separator\">foo barbaz</div>",
+      expectedSelectionRange: function (editor) {
+        return { collapsed: true,
+                 startContainer: editor.firstChild.firstChild,
+                 startOffset: 7 };
+      }, },
+    { description: "Joining <h1> and <p> elements",
+      innerHTML: "<h1>foo bar</h1><p id=\"separator\">baz</p>",
+      expectedInnerHTML: "<h1>foo barbaz</h1>",
+      expectedSelectionRange: function (editor) {
+        return { collapsed: true,
+                 startContainer: editor.firstChild.firstChild,
+                 startOffset: 7 };
+      }, },
+    { description: "Joining <h2> and <p> elements",
+      innerHTML: "<h2>foo bar</h2><p id=\"separator\">baz</p>",
+      expectedInnerHTML: "<h2>foo barbaz</h2>",
+      expectedSelectionRange: function (editor) {
+        return { collapsed: true,
+                 startContainer: editor.firstChild.firstChild,
+                 startOffset: 7 };
+      }, },
+    { description: "Joining <h3> and <p> elements",
+      innerHTML: "<h3>foo bar</h3><p id=\"separator\">baz</p>",
+      expectedInnerHTML: "<h3>foo barbaz</h3>",
+      expectedSelectionRange: function (editor) {
+        return { collapsed: true,
+                 startContainer: editor.firstChild.firstChild,
+                 startOffset: 7 };
+      }, },
+    { description: "Joining <h4> and <p> elements",
+      innerHTML: "<h4>foo bar</h4><p id=\"separator\">baz</p>",
+      expectedInnerHTML: "<h4>foo barbaz</h4>",
+      expectedSelectionRange: function (editor) {
+        return { collapsed: true,
+                 startContainer: editor.firstChild.firstChild,
+                 startOffset: 7 };
+      }, },
+    { description: "Joining <h5> and <p> elements",
+      innerHTML: "<h5>foo bar</h5><p id=\"separator\">baz</p>",
+      expectedInnerHTML: "<h5>foo barbaz</h5>",
+      expectedSelectionRange: function (editor) {
+        return { collapsed: true,
+                 startContainer: editor.firstChild.firstChild,
+                 startOffset: 7 };
+      }, },
+    { description: "Joining <h6> and <p> elements",
+      innerHTML: "<h6>foo bar</h6><p id=\"separator\">baz</p>",
+      expectedInnerHTML: "<h6>foo barbaz</h6>",
+      expectedSelectionRange: function (editor) {
+        return { collapsed: true,
+                 startContainer: editor.firstChild.firstChild,
+                 startOffset: 7 };
+      }, },
+    { description: "Joining <p> and <h1> elements",
+      innerHTML: "<p>foo bar</p><h1 id=\"separator\">baz</h1>",
+      expectedInnerHTML: "<p>foo barbaz</p>",
+      expectedSelectionRange: function (editor) {
+        return { collapsed: true,
+                 startContainer: editor.firstChild.firstChild,
+                 startOffset: 7 };
+      }, },
+    { description: "Joining <p> and <h2> elements",
+      innerHTML: "<p>foo bar</p><h2 id=\"separator\">baz</h2>",
+      expectedInnerHTML: "<p>foo barbaz</p>",
+      expectedSelectionRange: function (editor) {
+        return { collapsed: true,
+                 startContainer: editor.firstChild.firstChild,
+                 startOffset: 7 };
+      }, },
+    { description: "Joining <p> and <h3> elements",
+      innerHTML: "<p>foo bar</p><h3 id=\"separator\">baz</h3>",
+      expectedInnerHTML: "<p>foo barbaz</p>",
+      expectedSelectionRange: function (editor) {
+        return { collapsed: true,
+                 startContainer: editor.firstChild.firstChild,
+                 startOffset: 7 };
+      }, },
+    { description: "Joining <p> and <h4> elements",
+      innerHTML: "<p>foo bar</p><h4 id=\"separator\">baz</h4>",
+      expectedInnerHTML: "<p>foo barbaz</p>",
+      expectedSelectionRange: function (editor) {
+        return { collapsed: true,
+                 startContainer: editor.firstChild.firstChild,
+                 startOffset: 7 };
+      }, },
+    { description: "Joining <p> and <h5> elements",
+      innerHTML: "<p>foo bar</p><h5 id=\"separator\">baz</h5>",
+      expectedInnerHTML: "<p>foo barbaz</p>",
+      expectedSelectionRange: function (editor) {
+        return { collapsed: true,
+                 startContainer: editor.firstChild.firstChild,
+                 startOffset: 7 };
+      }, },
+    { description: "Joining <p> and <h6> elements",
+      innerHTML: "<p>foo bar</p><h6 id=\"separator\">baz</h6>",
+      expectedInnerHTML: "<p>foo barbar</p>",
+      expectedSelectionRange: function (editor) {
+        return { collapsed: true,
+                 startContainer: editor.firstChild.firstChild,
+                 startOffset: 7 };
+      }, },
+  ];
+
+  document.body.innerHTML = "<div id=\"editor\" contenteditable></div>";
+  let editor = document.getElementById("editor");
+  editor.focus();
+  let selection = document.getSelection();
+
+  for (const kTest of kTests) {
+    editor.innerHTML = kTest.innerHTML;
+    let separator = document.getElementById("separator");
+    function getFirstLeafNode(node) {
+      for (; node.firstChild; node = node.firstChild) {
+      }
+      return node;
+    }
+    if (separator.nodeName.toLowerCase() == "br") {
+      if (separator.nextSibling) {
+        selection.collapse(getFirstLeafNode(separator.nextSibling), 0);
+      } else {
+        selection.collapse(separator.parentNode,
+                           separator.parentNode.childNodes.length);
+      }
+    } else {
+      selection.collapse(getFirstLeafNode(separator), 0);
+    }
+    test(function () {
+      document.execCommand("delete", false);
+      assert_equals(editor.innerHTML, kTest.expectedInnerHTML);
+      const kExpectedSelectionRange = kTest.expectedSelectionRange(editor);
+      let range = selection.getRangeAt(0);
+      assert_equals(range.collapsed, kExpectedSelectionRange.collapsed);
+      assert_equals(range.startContainer, kExpectedSelectionRange.startContainer);
+      assert_equals(range.startOffset, kExpectedSelectionRange.startOffset);
+      if (kExpectedSelectionRange.collapsed) {
+        assert_equals(range.endContainer, kExpectedSelectionRange.startContainer);
+        assert_equals(range.endOffset, kExpectedSelectionRange.startOffset);
+      } else {
+        assert_equals(range.endContainer, kExpectedSelectionRange.endContainer);
+        assert_equals(range.endOffset, kExpectedSelectionRange.endOffset);
+      }
+    }, kTest.description + ", delete command");
+
+    editor.innerHTML = kTest.innerHTML;
+    separator = document.getElementById("separator");
+    function getLastLeafNode(node) {
+      for (; node.lastChild; node = node.lastChild) {
+      }
+      return node;
+    }
+    function getLength(node) {
+      if (node.length !== undefined) {
+        return node.length;
+      }
+      return node.childNodes.length;
+    }
+    if (separator.previousSibling) {
+      let lastLeafNode = getLastLeafNode(separator.previousSibling);
+      selection.collapse(lastLeafNode, getLength(lastLeafNode));
+    } else {
+      selection.collapse(separator.parentNode, 0);
+    }
+    test(function () {
+      try {
+        document.execCommand("forwarddelete", false);
+      } catch (e) {
+        console.log(e);
+      }
+      assert_equals(editor.innerHTML, kTest.expectedInnerHTML);
+      const kExpectedSelectionRange = kTest.expectedSelectionRange(editor);
+      let range = selection.getRangeAt(0);
+      assert_equals(range.collapsed, kExpectedSelectionRange.collapsed);
+      assert_equals(range.startContainer, kExpectedSelectionRange.startContainer);
+      assert_equals(range.startOffset, kExpectedSelectionRange.startOffset);
+      if (kExpectedSelectionRange.collapsed) {
+        assert_equals(range.endContainer, kExpectedSelectionRange.startContainer);
+        assert_equals(range.endOffset, kExpectedSelectionRange.startOffset);
+      } else {
+        assert_equals(range.endContainer, kExpectedSelectionRange.endContainer);
+        assert_equals(range.endOffset, kExpectedSelectionRange.endOffset);
+      }
+    }, kTest.description + ", forwarddelete command");
+  }
+})();
+</script>
+</body>
+</html>