Bug 1353695 - Sometimes Enter is ignored in editor; r=masayuki
authorAryeh Gregor <ayg@aryeh.name>
Wed, 05 Apr 2017 20:40:13 +0300
changeset 353600 f25386cf25adbe88123017699d984cec49e4d640
parent 353599 a649627e9ad1363e444d80279426f31dc4a0971a
child 353601 adc83a5de17d7061aeed65b9091c5f051c0c30a2
push id31673
push userkwierso@gmail.com
push dateTue, 18 Apr 2017 21:23:54 +0000
treeherdermozilla-central@1a81aadc2510 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmasayuki
bugs1353695
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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