Bug 1453872 - Make HTMLEditRules::JoinNodesSmart() return { aRightNode - aLeftNode.Length() } by default r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 13 Apr 2018 13:18:13 +0900
changeset 781845 3f52e182c2a5960aee83d0b5e2390dc6c98e335f
parent 781489 da809ecceaf3a8ada0aa2d7115822d39d0439654
push id106429
push usermasayuki@d-toybox.com
push dateFri, 13 Apr 2018 18:30:43 +0000
reviewersm_kato
bugs1453872, 1423835
milestone61.0a1
Bug 1453872 - Make HTMLEditRules::JoinNodesSmart() return { aRightNode - aLeftNode.Length() } by default r?m_kato 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/meta/MANIFEST.json
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
@@ -7779,18 +7779,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/meta/MANIFEST.json
+++ b/testing/web-platform/meta/MANIFEST.json
@@ -503532,17 +503532,17 @@
    "0cba1aed016d08e4706bffb8a4f4169c9cfd2108",
    "visual"
   ],
   "css/css-overflow/input-scrollable-region-001.html": [
    "f51bc673da28b0471018cdf945b4449ab00ce717",
    "reftest"
   ],
   "css/css-overflow/overflow-shorthand-001.html": [
-   "6409ee499d3e853d8f4933f1b532e12ed9ab406b",
+   "a32d1b270f62b9d563ed397c2c4cd6e87b9405e1",
    "testharness"
   ],
   "css/css-overflow/reference/input-scrollable-region-001-ref.html": [
    "31e24bb1a2cb6f42703cc05e055fcb345c770a22",
    "support"
   ],
   "css/css-page/OWNERS": [
    "d4ee6029cc9c064e0e8b2c76becf3b59c4f7b62b",
@@ -549600,17 +549600,17 @@
    "3099fb27913f11a983f955cb2a883a882911bfe4",
    "support"
   ],
   "fetch/api/request/destination/fetch-destination-iframe.https.html": [
    "8b9f5f87086914fa8964702042a21bb833ff2d54",
    "testharness"
   ],
   "fetch/api/request/destination/fetch-destination-no-load-event.https.html": [
-   "df03c7ad35e9fcae0287c57d9fd18f741a35833c",
+   "343618faf06d5809b0f782191566b36037277c31",
    "testharness"
   ],
   "fetch/api/request/destination/fetch-destination-worker.https.html": [
    "65fc76503d95a359bf2fafebe603aa19bd0a2bfb",
    "testharness"
   ],
   "fetch/api/request/destination/fetch-destination.https.html": [
    "5b7276e8a10bf91ee7d2a92917176b8e62c3255d",
@@ -549660,17 +549660,17 @@
    "da39a3ee5e6b4b0d3255bfef95601890afd80709",
    "support"
   ],
   "fetch/api/request/destination/resources/fetch-destination-worker-iframe.js": [
    "e3629357f9d0e3f93659aff23ab3138cc7978823",
    "support"
   ],
   "fetch/api/request/destination/resources/fetch-destination-worker-no-load-event.js": [
-   "26aedd2a95b90728af7aaa3dc9c5d11153343a7e",
+   "894f421753d61685faec672219964b599c7016f8",
    "support"
   ],
   "fetch/api/request/destination/resources/fetch-destination-worker.js": [
    "e10bc6423b2ee29387b1153546ae765449aa1ae4",
    "support"
   ],
   "fetch/api/request/destination/resources/importer.js": [
    "cebbf26e5c2237b62a1b5b01eb434dbfa405d28c",
--- 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>