Bug 1353695 - Sometimes Enter is ignored in editor; r=masayuki draft
authorAryeh Gregor <ayg@aryeh.name>
Wed, 05 Apr 2017 20:40:13 +0300
changeset 564197 8c65ed8eab81999937485d0ff89999c20b3a2f92
parent 556265 3854bcf837a729374b4d345910c274fed12cffe4
child 624703 aefbfd44de7b89533258495d3166f49c1d55141b
push id54561
push userbmo:ayg@aryeh.name
push dateTue, 18 Apr 2017 12:04:05 +0000
reviewersmasayuki
bugs1353695
milestone55.0a1
Bug 1353695 - Sometimes Enter is ignored in editor; r=masayuki When defaultParagraphSeparator is not "br", and we hit Enter on a line that is not contained in any block element, we first create a new <div> (or <p>) wrapper to hold the line's contents. If creating this wrapper fails for some reason, we go ahead and insert a <br> instead. In some cases, we would get confused and think we didn't create the block element when really we did. We would insert a <br>, and afterwards something would get rid of the empty block element. In a corner case where the line only consisted of a <br> to start with, this would result in nothing happening, because the original <br> was removed when creating the block element, and only one <br> was inserted to replace it. The correct fix is to just not get confused! MozReview-Commit-ID: 1U8KHC71bfw
editor/libeditor/HTMLEditRules.cpp
editor/libeditor/tests/test_bug780035.html
testing/web-platform/meta/editing/run/insertparagraph.html.ini
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -1565,16 +1565,25 @@ HTMLEditRules::WillInsertBreak(Selection
     // Insert a new block first
     MOZ_ASSERT(separator == ParagraphSeparator::div ||
                separator == ParagraphSeparator::p);
     nsresult rv = MakeBasicBlock(aSelection,
                                  ParagraphSeparatorElement(separator));
     // We warn on failure, but don't handle it, because it might be harmless.
     // Instead we just check that a new block was actually created.
     Unused << NS_WARN_IF(NS_FAILED(rv));
+
+    // Reinitialize node/offset in case they're not inside the new block
+    if (NS_WARN_IF(!aSelection.GetRangeAt(0) ||
+                   !aSelection.GetRangeAt(0)->GetStartParent())) {
+      return NS_ERROR_FAILURE;
+    }
+    node = *aSelection.GetRangeAt(0)->GetStartParent();
+    offset = aSelection.GetRangeAt(0)->StartOffset();
+
     blockParent = mHTMLEditor->GetBlock(node);
     if (NS_WARN_IF(!blockParent)) {
       return NS_ERROR_UNEXPECTED;
     }
     if (NS_WARN_IF(blockParent == host)) {
       // Didn't create a new block for some reason, fall back to <br>
       rv = StandardBreakImpl(node, offset, aSelection);
       if (NS_WARN_IF(NS_FAILED(rv))) {
--- a/editor/libeditor/tests/test_bug780035.html
+++ b/editor/libeditor/tests/test_bug780035.html
@@ -10,13 +10,13 @@ https://bugzilla.mozilla.org/show_bug.cg
 <div contenteditable style="font-size: 13.3333px"></div>
 <script>
 SimpleTest.waitForExplicitFinish();
 SimpleTest.waitForFocus(function() {
   document.querySelector("div").focus();
   document.execCommand("stylewithcss", false, true);
   sendKey("RETURN");
   sendChar("x");
-  is(document.querySelector("div").innerHTML, "x<br>",
-     "No <font> tag should be generated");
+  is(document.querySelector("div").innerHTML,
+     "<div><br></div><div>x<br></div>", "No <font> tag should be generated");
   SimpleTest.finish();
 });
 </script>
--- a/testing/web-platform/meta/editing/run/insertparagraph.html.ini
+++ b/testing/web-platform/meta/editing/run/insertparagraph.html.ini
@@ -16,22 +16,16 @@
     expected: FAIL
 
   [[["defaultparagraphseparator","div"\],["insertparagraph",""\]\] "<table><tr><td>fo[o<td>b\]ar<td>baz</table>" compare innerHTML]
     expected: FAIL
 
   [[["defaultparagraphseparator","p"\],["insertparagraph",""\]\] "<table><tr><td>fo[o<td>b\]ar<td>baz</table>" compare innerHTML]
     expected: FAIL
 
-  [[["defaultparagraphseparator","div"\],["insertparagraph",""\]\] "{<table><tr><td>foo</table>}" compare innerHTML]
-    expected: FAIL
-
-  [[["defaultparagraphseparator","p"\],["insertparagraph",""\]\] "{<table><tr><td>foo</table>}" compare innerHTML]
-    expected: FAIL
-
   [[["defaultparagraphseparator","div"\],["insertparagraph",""\]\] "<table><tr><td>[foo\]</table>" compare innerHTML]
     expected: FAIL
 
   [[["defaultparagraphseparator","p"\],["insertparagraph",""\]\] "<table><tr><td>[foo\]</table>" compare innerHTML]
     expected: FAIL
 
   [[["defaultparagraphseparator","div"\],["insertparagraph",""\]\] "<span>foo[\]</span>" compare innerHTML]
     expected: FAIL
@@ -49,46 +43,16 @@
     expected: FAIL
 
   [[["insertparagraph",""\]\] "<dl><dt>foo<dd>bar<dl><dt>{}<br><dd>baz</dl></dl>" compare innerHTML]
     expected: FAIL
 
   [[["insertparagraph",""\]\] "<dl><dt>foo<dd>bar<dl><dt>baz<dd>{}<br></dl></dl>" compare innerHTML]
     expected: FAIL
 
-  [[["defaultparagraphseparator","div"\],["insertparagraph",""\]\] "<p>foo</p>{}<br>" compare innerHTML]
-    expected: FAIL
-
-  [[["defaultparagraphseparator","p"\],["insertparagraph",""\]\] "<p>foo</p>{}<br>" compare innerHTML]
-    expected: FAIL
-
-  [[["defaultparagraphseparator","div"\],["insertparagraph",""\]\] "{}<br><p>foo</p>" compare innerHTML]
-    expected: FAIL
-
-  [[["defaultparagraphseparator","p"\],["insertparagraph",""\]\] "{}<br><p>foo</p>" compare innerHTML]
-    expected: FAIL
-
-  [[["defaultparagraphseparator","div"\],["insertparagraph",""\]\] "<p>foo</p>{}<br><h1>bar</h1>" compare innerHTML]
-    expected: FAIL
-
-  [[["defaultparagraphseparator","p"\],["insertparagraph",""\]\] "<p>foo</p>{}<br><h1>bar</h1>" compare innerHTML]
-    expected: FAIL
-
-  [[["defaultparagraphseparator","div"\],["insertparagraph",""\]\] "<h1>foo</h1>{}<br><p>bar</p>" compare innerHTML]
-    expected: FAIL
-
-  [[["defaultparagraphseparator","p"\],["insertparagraph",""\]\] "<h1>foo</h1>{}<br><p>bar</p>" compare innerHTML]
-    expected: FAIL
-
-  [[["defaultparagraphseparator","div"\],["insertparagraph",""\]\] "<h1>foo</h1>{}<br><h2>bar</h2>" compare innerHTML]
-    expected: FAIL
-
-  [[["defaultparagraphseparator","p"\],["insertparagraph",""\]\] "<h1>foo</h1>{}<br><h2>bar</h2>" compare innerHTML]
-    expected: FAIL
-
   [[["defaultparagraphseparator","div"\],["insertparagraph",""\]\] "<p>foo</p>{<h1>bar</h1>}<p>baz</p>" compare innerHTML]
     expected: FAIL
 
   [[["defaultparagraphseparator","p"\],["insertparagraph",""\]\] "<p>foo</p>{<h1>bar</h1>}<p>baz</p>" compare innerHTML]
     expected: FAIL
 
   [[["defaultparagraphseparator","div"\],["insertparagraph",""\]\] "<table><tr><td>foo[\]bar</table>" compare innerHTML]
     expected: FAIL