Bug 1636747 - Backout the 1st patch of bug 503838 r=m_kato

Differential Revision: https://phabricator.services.mozilla.com/D75032

editor/libeditor/HTMLEditSubActionHandler.cpp

@@ -1700,38 +1700,6 @@ EditActionResult HTMLEditor::HandleInsertText(
                        "Failed to unset interline position");
 
   if (currentPoint.IsSet()) {
-    if (currentPoint.IsInTextNode() &&
-        IsVisibleTextNode(*currentPoint.ContainerAsText())) {
-      // If the text is visible, we should remove padding <br> element if
-      // there is.  Currently, we should remove it when it's immediately
-      // after the text node.  I.e., not scanning next <br> element outside
-      // the parent element even if there is one in same block because <br>
-      // element should be in same element if it's created for empty line.
-      RefPtr<nsIContent> nextSibling =
-          currentPoint.GetContainer()->GetNextSibling();
-      if (nextSibling && nextSibling->IsHTMLElement(nsGkAtoms::br) &&
-          !IsVisibleBRElement(nextSibling)) {
-        {
-          AutoTrackDOMPoint tracker(RangeUpdaterRef(), &currentPoint);
-          nsresult rv = DeleteNodeWithTransaction(*nextSibling);
-          if (NS_WARN_IF(Destroyed())) {
-            return EditActionHandled(NS_ERROR_EDITOR_DESTROYED);
-          }
-          if (NS_FAILED(rv)) {
-            NS_WARNING(
-                "HTMLEditor::DeleteNodeWithTransaction() failed to remove "
-                "unnecessary padding <br> element");
-            return EditActionHandled(rv);
-          }
-        }
-        if (!currentPoint.IsSetAndValid()) {
-          NS_WARNING(
-              "Mutation event listener changed the DOM tree unexpected while "
-              "removing invisible <br> element");
-          return EditActionHandled(NS_ERROR_EDITOR_UNEXPECTED_DOM_TREE);
-        }
-      }
-    }
     nsresult rv = CollapseSelectionTo(currentPoint);
     if (NS_WARN_IF(rv == NS_ERROR_EDITOR_DESTROYED)) {
       return EditActionHandled(NS_ERROR_EDITOR_DESTROYED);

editor/libeditor/tests/test_bug1250010.html

@@ -45,8 +45,8 @@ SimpleTest.waitForFocus(function() {
   sendString("a");
 
   is(div.innerHTML, "<p><b><font color=\"red\">1234567890</font></b></p>" +
-                    "<p><b><font color=\"red\">a</font></b></p>" +
-                    "<p><b><font color=\"red\">b</font></b></p>",
+                    "<p><b><font color=\"red\">a<br></font></b></p>" +
+                    "<p><b><font color=\"red\">b<br></font></b></p>",
                     "unexpected HTML");
 
   // Second test: Since we modified the code path that splits non-text nodes,

editor/libeditor/tests/test_bug1330796.html

@@ -41,10 +41,10 @@ var tests = [
   [ "<span _moz_quote=true style=\"display: block;\">&gt; mailcite<br></span>", 5,
     "<span style=\"display: block;\">&gt; mai<br></span>x<br><span style=\"display: block;\">lcite<br></span>"],
   [ "<span _moz_quote=true style=\"display: block;\">&gt; mailcite<br></span>", 10,
-    "<span style=\"display: block;\">&gt; mailcite<br></span>x" ],
+    "<span style=\"display: block;\">&gt; mailcite<br></span>x<br>" ],
   // No <br> at the end to simulate prior deletion to the end of the quote.
   [ "<span _moz_quote=true style=\"display: block;\">&gt; mailcite</span>", 10,
-    "<span style=\"display: block;\">&gt; mailcite<br></span>x" ],
+    "<span style=\"display: block;\">&gt; mailcite<br></span>x<br>" ],
 
   // Without style="display: block;".
   [ "<span _moz_quote=true>&gt; mailcite<br></span>", 0,
@@ -52,10 +52,10 @@ var tests = [
   [ "<span _moz_quote=true>&gt; mailcite<br></span>", 5,
     "<span>&gt; mai</span><br>x<br><span>lcite<br></span>" ],
   [ "<span _moz_quote=true>&gt; mailcite<br></span>", 10,
-    "<span>&gt; mailcite<br></span>x" ],
+    "<span>&gt; mailcite<br></span>x<br>" ],
   // No <br> at the end to simulate prior deletion to the end of the quote.
   [ "<span _moz_quote=true>&gt; mailcite</span>", 10,
-    "<span>&gt; mailcite</span><br>x" ],
+    "<span>&gt; mailcite</span><br>x<br>" ],
 ];
 
 /** Test for Bug 1330796 **/

editor/libeditor/tests/test_bug1385905.html

@@ -33,8 +33,9 @@ SimpleTest.waitForFocus(() => {
      "Typing Enter at start of the <div> element should split the <div> element");
   synthesizeKey("KEY_ArrowUp");
   sendString("x");
-  is(editor.innerHTML, "<div>x</div><div>contents</div>",
-     "Typing 'x' at the empty <div> element should just insert 'x' into the <div> element and remove the padding <br> element");
+  is(editor.innerHTML, "<div>x<br></div><div>contents</div>",
+     "Typing 'x' at the empty <div> element should just insert 'x' into the <div> element");
+  ensureNoPaddingBR();
   synthesizeKey("KEY_Enter");
   is(editor.innerHTML, "<div>x</div><div><br></div><div>contents</div>",
      "Typing Enter next to 'x' in the first <div> element should split the <div> element and inserts <br> element to a new <div> element");

editor/libeditor/tests/test_bug1397412.xhtml

@@ -28,7 +28,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=1397412
   <![CDATA[
 function runTest() {
   var initialHTML1 = "xx<br><br>";
-  var expectedHTML1 = "xx<br>t";
+  var expectedHTML1 = "xx<br>t<br>";
   var initialHTML2 = "xx<br><br>yy<br>";
   var expectedHTML2 = "xx<br>t<br>yy<br>";
   window.docShell
@@ -47,8 +47,7 @@ function runTest() {
   selection.collapse(body, 2);
   sendString("t");
   var actualHTML = body.innerHTML;
-  is(actualHTML, expectedHTML1,
-      "'t' should be inserted immediately after the first <br> and the other one should be removed");
+  is(actualHTML, expectedHTML1, "'t' should be inserted between <br>s");
 
   // Test 2.
   body.innerHTML = initialHTML2;

editor/libeditor/tests/test_bug551704.html

@@ -112,7 +112,7 @@ function continueTest() {
     } else {
       type = typeABCDEF;
     }
-    var expectedHTML = "<div>abc</div><div>def</div>";
+    var expectedHTML = "<div>abc</div><div>def<br></div>";
     var expectedText = "abc\ndef";
     testLineBreak(div, type, expectedText, expectedHTML, doNextTest);
   }

editor/libeditor/tests/test_bug780035.html

@@ -17,7 +17,7 @@ SimpleTest.waitForFocus(function() {
   sendKey("RETURN");
   sendChar("x");
   is(document.querySelector("div").innerHTML,
-     "<div><br></div><div>x</div>", "No <font> tag should be generated");
+     "<div><br></div><div>x<br></div>", "No <font> tag should be generated");
   SimpleTest.finish();
 });
 </script>

editor/libeditor/tests/test_bug832025.html

@@ -35,7 +35,7 @@ sendKey("return");
 // insert some text
 document.execCommand("insertText", false, "abc");
 
-is(test.innerHTML, "<h1>header1</h1><div>abc</div>",
+is(test.innerHTML, "<h1>header1</h1><div>abc<br></div>",
    "A paragraph automatically created after a CR at the end of an H1 should not be bold");
 
 </script>

editor/libeditor/tests/test_dom_input_event_on_htmleditor.html

@@ -225,7 +225,12 @@ async function runTests() {
     cancelBeforeInput = false;
     action = 'typing "B"';
     synthesizeKey("B", {shiftKey: true}, aWindow);
-    is(editTarget.innerHTML, "B", `${aDescription}"B" should've been inserted by ${action}`);
+    // XXX This inconsistency must be a bug.
+    if (!isEditTargetIsDescendantOfEditingHost) {
+      is(editTarget.innerHTML, "B<br>", `${aDescription}"B" should've been inserted by ${action}`);
+    } else {
+      is(editTarget.innerHTML, "B", `${aDescription}"B" should've been inserted by ${action}`);
+    }
     is(beforeInputEvent.cancelable, true, `${aDescription}"beforeinput" event for ${action} should be cancelable`);
     is(beforeInputEvent.inputType, "insertText", `${aDescription}inputType of "beforeinput" event for ${action} should be "insertText"`);
     is(beforeInputEvent.data, "B", `${aDescription}data of "beforeinput" event for ${action} should be "B"`);
@@ -241,7 +246,12 @@ async function runTests() {
     cancelBeforeInput = true;
     action = 'typing "Enter"';
     synthesizeKey("KEY_Enter", {}, aWindow);
-    is(editTarget.innerHTML, "B", `${aDescription}shouldn't modify the editor by ${action} since "beforeinput" was canceled`);
+    // XXX This inconsistency must be a bug.
+    if (!isEditTargetIsDescendantOfEditingHost) {
+      is(editTarget.innerHTML, "B<br>", `${aDescription}shouldn't modify the editor by ${action} since "beforeinput" was canceled`);
+    } else {
+      is(editTarget.innerHTML, "B", `${aDescription}shouldn't modify the editor by ${action} since "beforeinput" was canceled`);
+    }
     ok(beforeInputEvent, `${aDescription}"beforeinput" event should've been fired at ${action}`);
     ok(!inputEvent, `${aDescription}"input" event shouldn't been fired at ${action} since "beforeinput" was canceled`);
 
@@ -273,9 +283,9 @@ async function runTests() {
     action = 'typing "C" in new paragraph';
     synthesizeKey("C", {shiftKey: true}, aWindow);
     if (!isEditTargetIsDescendantOfEditingHost) {
-      is(editTarget.innerHTML, "<div>B</div><div>C</div>", `${aDescription}should insert "C" into the new paragraph by ${action}`);
+      is(editTarget.innerHTML, "<div>B</div><div>C<br></div>", `${aDescription}should insert "C" into the new paragraph by ${action}`);
     } else {
-      is(editTarget.innerHTML, "B<br>C", `${aDescription}should insert "C" into the new paragraph by ${action}`);
+      is(editTarget.innerHTML, "B<br>C<br>", `${aDescription}should insert "C" into the new paragraph by ${action}`);
     }
     is(beforeInputEvent.cancelable, true, `${aDescription}"beforeinput" event for ${action} should be cancelable`);
     is(beforeInputEvent.inputType, "insertText", `${aDescription}inputType of "beforeinput" event for ${action} should be "insertText"`);

editor/libeditor/tests/test_dragdrop.html

@@ -1613,7 +1613,7 @@ async function doTest() {
   });
   is(input.value, "Somt",
      `${description}: dragged range should be removed from <input>`);
-  is(contenteditable.innerHTML, "e Tex",
+  is(contenteditable.innerHTML, "e Tex<br>",
      `${description}: dragged content should be inserted into contenteditable`);
   is(beforeinputEvents.length, 2,
      `${description}: 2 "beforeinput" events should be fired on <input> and contenteditable`);
@@ -1659,7 +1659,7 @@ async function doTest() {
   });
   is(input.value, "Some Text",
      `${description}: dragged range shouldn't be removed from <input>`);
-  is(contenteditable.innerHTML, "e Tex",
+  is(contenteditable.innerHTML, "e Tex<br>",
      `${description}: dragged content should be inserted into contenteditable`);
   is(beforeinputEvents.length, 2,
      `${description}: 2 "beforeinput" events should be fired on <input> and contenteditable`);
@@ -1748,7 +1748,7 @@ async function doTest() {
   });
   is(input.value, "Some Text",
      `${description}: dragged range shouldn't be removed from <input>`);
-  is(contenteditable.innerHTML, "e Tex",
+  is(contenteditable.innerHTML, "e Tex<br>",
      `${description}: dragged content should be inserted into contenteditable`);
   is(beforeinputEvents.length, 1,
      `${description}: only one "beforeinput" events should be fired on contenteditable`);
@@ -1793,7 +1793,7 @@ async function doTest() {
      `${description}: dragged range should be removed from <textarea>`);
   todo_is(contenteditable.innerHTML, "<div>e1</div><div>Li</div>",
      `${description}: dragged content should be inserted into contenteditable`);
-  todo_isnot(contenteditable.innerHTML, "e1<br>Li",
+  todo_isnot(contenteditable.innerHTML, "e1<br>Li<br>",
      `${description}: dragged content should be inserted into contenteditable`);
   is(beforeinputEvents.length, 2,
      `${description}: 2 "beforeinput" events should be fired on <input> and contenteditable`);
@@ -1841,7 +1841,7 @@ async function doTest() {
      `${description}: dragged range should be removed from <textarea>`);
   todo_is(contenteditable.innerHTML, "<div>e1</div><div>Li</div>",
      `${description}: dragged content should be inserted into contenteditable`);
-  todo_isnot(contenteditable.innerHTML, "e1<br>Li",
+  todo_isnot(contenteditable.innerHTML, "e1<br>Li<br>",
      `${description}: dragged content should be inserted into contenteditable`);
   is(beforeinputEvents.length, 1,
      `${description}: only one "beforeinput" events should be fired on contenteditable`);

testing/web-platform/meta/editing/run/inserttext.html.ini

@@ -130,6 +130,12 @@
   [[["inserttext"," "\]\] " <span> </span>[\]foo" compare innerHTML]
     expected: FAIL
 
+  [[["inserttext"," "\]\] "{}<br>" compare innerHTML]
+    expected: FAIL
+
+  [[["inserttext"," "\]\] "<p>{}<br>" compare innerHTML]
+    expected: FAIL
+
   [[["inserttext"," "\]\] "<p>foo[\]<p>bar" compare innerHTML]
     expected: FAIL
 
@@ -234,6 +240,15 @@
   [[["defaultparagraphseparator","div"\],["inserttext","a"\]\] "<p>fo[o<p>b\]ar" queryCommandValue("defaultparagraphseparator") before]
     expected: FAIL
 
+  [[["inserttext","a"\]\] "{}<br>" compare innerHTML]
+    expected: FAIL
+
+  [[["inserttext","a"\]\] "<p>{}<br>" compare innerHTML]
+    expected: FAIL
+
+  [[["inserttext","a"\]\] "<p><span>{}<br></span>" compare innerHTML]
+    expected: FAIL
+
   [[["inserttext","a"\]\] "<p>foo{<span style=color:#aBcDeF>bar</span>}baz" compare innerHTML]
     expected: FAIL
 

widget/tests/window_composition_text_querycontent.xhtml

@@ -8839,8 +8839,7 @@ async function runIMEContentObserverTest()
     synthesizeKey("KEY_Backspace", {}, win, callback);
     await waitNotifications;
     ensureToRemovePrecedingPositionChangeNotification();
-    // Due to becoming the editor empty, <br> element needs to be appended.  Therefore, addedLength has to be kLFLen.
-    checkTextChangeNotification(notifications[0], description, { offset: 0 + offsetAtContainer, removedLength: 2, addedLength: kLFLen });
+    checkTextChangeNotification(notifications[0], description, { offset: 0 + offsetAtContainer, removedLength: 2, addedLength: 0 });
     checkSelectionChangeNotification(notifications[1], description, { offset: 0 + offsetAtContainer, text: "" });
     checkPositionChangeNotification(notifications[2], description);
     dumpUnexpectedNotifications(description, 3);
@@ -8974,10 +8973,11 @@ async function runIMEContentObserverTest()
     aElement.innerHTML = "<div>1<div>2<div>3</div>4</div>5</div>";
     await waitNotifications;
     ensureToRemovePrecedingPositionChangeNotification();
+    // There is <br> after the end of the line.  Therefore, removed length includes a line breaker length.
     if (isDefaultParagraphSeparatorBlock) {
-      checkTextChangeNotification(notifications[0], description, { offset: 0 + offsetAtContainer - kLFLen, removedLength: getNativeText("\nad").length, addedLength: getNativeText("\n1\n2\n345").length });
+      checkTextChangeNotification(notifications[0], description, { offset: 0 + offsetAtContainer - kLFLen, removedLength: getNativeText("\nad\n").length, addedLength: getNativeText("\n1\n2\n345").length });
     } else {
-      checkTextChangeNotification(notifications[0], description, { offset: 0 + offsetAtContainer, removedLength: "ad".length, addedLength: getNativeText("\n1\n2\n345").length });
+      checkTextChangeNotification(notifications[0], description, { offset: 0 + offsetAtContainer, removedLength: 2 + kLFLen, addedLength: getNativeText("\n1\n2\n345").length });
     }
     checkSelectionChangeNotification(notifications[1], description, { offset: 0, text: "" });
     checkPositionChangeNotification(notifications[2], description);