Crash in ReplaceSelectionCommand::mergeEndIfNeeded()

Julian Gonzalez · webkit-commit-queue · commit 7c7310e01f9e · 2021-04-15T22:34:33.000Z
https://bugs.webkit.org/show_bug.cgi?id=224587 Patch by Julian Gonzalez <julian_a_gonzalez@apple.com> on 2021-04-15 Reviewed by Ryosuke Niwa. Source/WebCore: Return early in mergeEndIfNeeded() if either of our VisiblePositions are null. Test: editing/inserting/insert-horizontal-rule-with-style-crash.html * editing/ReplaceSelectionCommand.cpp: (WebCore::ReplaceSelectionCommand::mergeEndIfNeeded): LayoutTests: Add a test to catch the editing crash fixed here; thanks to Tuomas Karkkainen for its basic structure and Ryosuke Niwa for some fixes. * editing/inserting/insert-horizontal-rule-with-style-crash-expected.txt: Added. * editing/inserting/insert-horizontal-rule-with-style-crash.html: Added. Canonical link: https://commits.webkit.org/236591@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@276078 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2021-04-15  Julian Gonzalez  <julian_a_gonzalez@apple.com>
+
+        Crash in ReplaceSelectionCommand::mergeEndIfNeeded()
+        https://bugs.webkit.org/show_bug.cgi?id=224587
+
+        Reviewed by Ryosuke Niwa.
+
+        Add a test to catch the editing crash fixed here; thanks to Tuomas Karkkainen
+        for its basic structure and Ryosuke Niwa for some fixes.
+
+        * editing/inserting/insert-horizontal-rule-with-style-crash-expected.txt: Added.
+        * editing/inserting/insert-horizontal-rule-with-style-crash.html: Added.
+
 2021-04-15  Antoine Quint  <graouts@webkit.org>
 
         calc() simplification for a multiplication should apply the multiplication to each value of an addition
diff --git a/LayoutTests/editing/inserting/insert-horizontal-rule-with-style-crash-expected.txt b/LayoutTests/editing/inserting/insert-horizontal-rule-with-style-crash-expected.txt
@@ -0,0 +1 @@
+This test passes if it does not crash. PASS
diff --git a/LayoutTests/editing/inserting/insert-horizontal-rule-with-style-crash.html b/LayoutTests/editing/inserting/insert-horizontal-rule-with-style-crash.html
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style></style>
+<title>InsertHRWithStyle</title>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+onload = () => {
+    document.styleSheets[0].insertRule(`:not(:read-only) { display: inline-table; }`);
+    document.styleSheets[0].insertRule(`:not(:host) { border-top: 1px solid; }`);
+    document.styleSheets[0].insertRule(`:last-of-type { all: unset; }`);
+    document.querySelector('title').appendChild(document.createElement('div'));
+    document.execCommand('SelectAll');
+    document.designMode = 'on';
+    document.execCommand('InsertHorizontalRule');
+    document.head.replaceChildren();
+    document.body.textContent = 'This test passes if it does not crash. PASS';
+};
+</script>
+<script></script>
+</head>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,17 @@
+2021-04-15  Julian Gonzalez  <julian_a_gonzalez@apple.com>
+
+        Crash in ReplaceSelectionCommand::mergeEndIfNeeded()
+        https://bugs.webkit.org/show_bug.cgi?id=224587
+
+        Reviewed by Ryosuke Niwa.
+
+        Return early in mergeEndIfNeeded() if either of our VisiblePositions are null.
+
+        Test: editing/inserting/insert-horizontal-rule-with-style-crash.html
+
+        * editing/ReplaceSelectionCommand.cpp:
+        (WebCore::ReplaceSelectionCommand::mergeEndIfNeeded):
+
 2021-04-15  Manuel Rego Casasnovas  <rego@igalia.com>
 
         Move FocusRemovalEventsMode into FocusOptions
diff --git a/Source/WebCore/editing/ReplaceSelectionCommand.cpp b/Source/WebCore/editing/ReplaceSelectionCommand.cpp
@@ -990,6 +990,10 @@ void ReplaceSelectionCommand::mergeEndIfNeeded()
         ASSERT_NOT_REACHED();
         return;
     }
+
+    ASSERT(startOfInsertedContent.isNull() == endOfInsertedContent.isNull());
+    if (startOfInsertedContent.isNull() || endOfInsertedContent.isNull())
+        return;
     
     // Merging two paragraphs will destroy the moved one's block styles.  Always move the end of inserted forward 
     // to preserve the block style of the paragraph already in the document, unless the paragraph to move would 

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+This test passes if it does not crash. PASS`