- Revision
- 266547
- Author
- alanc...@apple.com
- Date
- 2020-09-03 14:30:50 -0700 (Thu, 03 Sep 2020)
Log Message
Cherry-pick r266248. rdar://problem/67963541
Occasional crashes when restoring replaced text under Editor::changeBackToReplacedString
https://bugs.webkit.org/show_bug.cgi?id=215892
<rdar://problem/67731156>
Reviewed by Tim Horton.
Source/WebCore:
While reverting the replaced string in `Editor::changeBackToReplacedString`, it's possible for text replacement
(`Editor::replaceSelectionWithText`) to cause the text checking paragraph range to become orphaned (if for no
reason other than the fact that arbitrary script can run in between). If this happens, then our attempts to
expand the checking range to paragraph boundaries underneath `TextCheckingParagraph::paragraphRange` will result
in a null dereference, since the boundary points computed underneath the `expandToParagraphBoundary` helper will
be `nullopt`.
Mitigate this (and any other potentially similar crashes) by making `expandToParagraphBoundary` robust in the
case where expanding to the start and end fails, and just fall back to returning the original text checking
range instead.
Test: editing/mac/input/change-back-to-replaced-string.html
* editing/Editor.h:
* editing/TextCheckingHelper.cpp:
(WebCore::expandToParagraphBoundary):
See above for more details.
* testing/Internals.cpp:
(WebCore::Internals::changeBackToReplacedString):
* testing/Internals.h:
* testing/Internals.idl:
Add a simple internal testing hook to change the currently selected text back to the given replaced string.
LayoutTests:
Add a layout test to exercise the crash.
* editing/mac/input/change-back-to-replaced-string-expected.txt: Added.
* editing/mac/input/change-back-to-replaced-string.html: Added.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@266248 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Added Paths
Diff
Modified: branches/safari-610-branch/LayoutTests/ChangeLog (266546 => 266547)
--- branches/safari-610-branch/LayoutTests/ChangeLog 2020-09-03 21:30:46 UTC (rev 266546)
+++ branches/safari-610-branch/LayoutTests/ChangeLog 2020-09-03 21:30:50 UTC (rev 266547)
@@ -1,5 +1,66 @@
2020-09-03 Alan Coon <alanc...@apple.com>
+ Cherry-pick r266248. rdar://problem/67963541
+
+ Occasional crashes when restoring replaced text under Editor::changeBackToReplacedString
+ https://bugs.webkit.org/show_bug.cgi?id=215892
+ <rdar://problem/67731156>
+
+ Reviewed by Tim Horton.
+
+ Source/WebCore:
+
+ While reverting the replaced string in `Editor::changeBackToReplacedString`, it's possible for text replacement
+ (`Editor::replaceSelectionWithText`) to cause the text checking paragraph range to become orphaned (if for no
+ reason other than the fact that arbitrary script can run in between). If this happens, then our attempts to
+ expand the checking range to paragraph boundaries underneath `TextCheckingParagraph::paragraphRange` will result
+ in a null dereference, since the boundary points computed underneath the `expandToParagraphBoundary` helper will
+ be `nullopt`.
+
+ Mitigate this (and any other potentially similar crashes) by making `expandToParagraphBoundary` robust in the
+ case where expanding to the start and end fails, and just fall back to returning the original text checking
+ range instead.
+
+ Test: editing/mac/input/change-back-to-replaced-string.html
+
+ * editing/Editor.h:
+ * editing/TextCheckingHelper.cpp:
+ (WebCore::expandToParagraphBoundary):
+
+ See above for more details.
+
+ * testing/Internals.cpp:
+ (WebCore::Internals::changeBackToReplacedString):
+ * testing/Internals.h:
+ * testing/Internals.idl:
+
+ Add a simple internal testing hook to change the currently selected text back to the given replaced string.
+
+ LayoutTests:
+
+ Add a layout test to exercise the crash.
+
+ * editing/mac/input/change-back-to-replaced-string-expected.txt: Added.
+ * editing/mac/input/change-back-to-replaced-string.html: Added.
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@266248 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2020-08-27 Wenson Hsieh <wenson_hs...@apple.com>
+
+ Occasional crashes when restoring replaced text under Editor::changeBackToReplacedString
+ https://bugs.webkit.org/show_bug.cgi?id=215892
+ <rdar://problem/67731156>
+
+ Reviewed by Tim Horton.
+
+ Add a layout test to exercise the crash.
+
+ * editing/mac/input/change-back-to-replaced-string-expected.txt: Added.
+ * editing/mac/input/change-back-to-replaced-string.html: Added.
+
+2020-09-03 Alan Coon <alanc...@apple.com>
+
Cherry-pick r266239. rdar://problem/67963562
REGRESSION (r258215): Title preview movie isn't displayed at www.thismmalife.com
Added: branches/safari-610-branch/LayoutTests/editing/mac/input/change-back-to-replaced-string-expected.txt (0 => 266547)
--- branches/safari-610-branch/LayoutTests/editing/mac/input/change-back-to-replaced-string-expected.txt (rev 0)
+++ branches/safari-610-branch/LayoutTests/editing/mac/input/change-back-to-replaced-string-expected.txt 2020-09-03 21:30:50 UTC (rev 266547)
@@ -0,0 +1,11 @@
+The test passes if we avoid crashing when removing the editable root while restoring replaced text. This test requires WebKitTestRunner or DumpRenderTree.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Removed editor.
+PASS Changed back to original text without crashing.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: branches/safari-610-branch/LayoutTests/editing/mac/input/change-back-to-replaced-string.html (0 => 266547)
--- branches/safari-610-branch/LayoutTests/editing/mac/input/change-back-to-replaced-string.html (rev 0)
+++ branches/safari-610-branch/LayoutTests/editing/mac/input/change-back-to-replaced-string.html 2020-09-03 21:30:50 UTC (rev 266547)
@@ -0,0 +1,32 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <script src=""
+ <script src=""
+</head>
+<body>
+ <div id="editor" contenteditable></div>
+ <div id="console"></div>
+ <script>
+ internals.settings.setUnifiedTextCheckerEnabled(true);
+ description("The test passes if we avoid crashing when removing the editable root while restoring replaced text. This test requires WebKitTestRunner or DumpRenderTree.");
+
+ let editor = document.getElementById("editor");
+ editor.focus();
+
+ for (let character of "hello ")
+ typeCharacterCommand(character);
+
+ let textNode = editor.childNodes[0];
+ getSelection().setBaseAndExtent(textNode, 0, textNode, 5);
+
+ editor.addEventListener("input", event => {
+ editor.remove();
+ testPassed("Removed editor.");
+ });
+
+ internals.changeBackToReplacedString("helo");
+ testPassed("Changed back to original text without crashing.");
+ </script>
+</body>
+</html>
\ No newline at end of file
Modified: branches/safari-610-branch/Source/WebCore/ChangeLog (266546 => 266547)
--- branches/safari-610-branch/Source/WebCore/ChangeLog 2020-09-03 21:30:46 UTC (rev 266546)
+++ branches/safari-610-branch/Source/WebCore/ChangeLog 2020-09-03 21:30:50 UTC (rev 266547)
@@ -1,5 +1,87 @@
2020-09-03 Alan Coon <alanc...@apple.com>
+ Cherry-pick r266248. rdar://problem/67963541
+
+ Occasional crashes when restoring replaced text under Editor::changeBackToReplacedString
+ https://bugs.webkit.org/show_bug.cgi?id=215892
+ <rdar://problem/67731156>
+
+ Reviewed by Tim Horton.
+
+ Source/WebCore:
+
+ While reverting the replaced string in `Editor::changeBackToReplacedString`, it's possible for text replacement
+ (`Editor::replaceSelectionWithText`) to cause the text checking paragraph range to become orphaned (if for no
+ reason other than the fact that arbitrary script can run in between). If this happens, then our attempts to
+ expand the checking range to paragraph boundaries underneath `TextCheckingParagraph::paragraphRange` will result
+ in a null dereference, since the boundary points computed underneath the `expandToParagraphBoundary` helper will
+ be `nullopt`.
+
+ Mitigate this (and any other potentially similar crashes) by making `expandToParagraphBoundary` robust in the
+ case where expanding to the start and end fails, and just fall back to returning the original text checking
+ range instead.
+
+ Test: editing/mac/input/change-back-to-replaced-string.html
+
+ * editing/Editor.h:
+ * editing/TextCheckingHelper.cpp:
+ (WebCore::expandToParagraphBoundary):
+
+ See above for more details.
+
+ * testing/Internals.cpp:
+ (WebCore::Internals::changeBackToReplacedString):
+ * testing/Internals.h:
+ * testing/Internals.idl:
+
+ Add a simple internal testing hook to change the currently selected text back to the given replaced string.
+
+ LayoutTests:
+
+ Add a layout test to exercise the crash.
+
+ * editing/mac/input/change-back-to-replaced-string-expected.txt: Added.
+ * editing/mac/input/change-back-to-replaced-string.html: Added.
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@266248 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2020-08-27 Wenson Hsieh <wenson_hs...@apple.com>
+
+ Occasional crashes when restoring replaced text under Editor::changeBackToReplacedString
+ https://bugs.webkit.org/show_bug.cgi?id=215892
+ <rdar://problem/67731156>
+
+ Reviewed by Tim Horton.
+
+ While reverting the replaced string in `Editor::changeBackToReplacedString`, it's possible for text replacement
+ (`Editor::replaceSelectionWithText`) to cause the text checking paragraph range to become orphaned (if for no
+ reason other than the fact that arbitrary script can run in between). If this happens, then our attempts to
+ expand the checking range to paragraph boundaries underneath `TextCheckingParagraph::paragraphRange` will result
+ in a null dereference, since the boundary points computed underneath the `expandToParagraphBoundary` helper will
+ be `nullopt`.
+
+ Mitigate this (and any other potentially similar crashes) by making `expandToParagraphBoundary` robust in the
+ case where expanding to the start and end fails, and just fall back to returning the original text checking
+ range instead.
+
+ Test: editing/mac/input/change-back-to-replaced-string.html
+
+ * editing/Editor.h:
+ * editing/TextCheckingHelper.cpp:
+ (WebCore::expandToParagraphBoundary):
+
+ See above for more details.
+
+ * testing/Internals.cpp:
+ (WebCore::Internals::changeBackToReplacedString):
+ * testing/Internals.h:
+ * testing/Internals.idl:
+
+ Add a simple internal testing hook to change the currently selected text back to the given replaced string.
+
+2020-09-03 Alan Coon <alanc...@apple.com>
+
Cherry-pick r266239. rdar://problem/67963562
REGRESSION (r258215): Title preview movie isn't displayed at www.thismmalife.com
Modified: branches/safari-610-branch/Source/WebCore/editing/Editor.h (266546 => 266547)
--- branches/safari-610-branch/Source/WebCore/editing/Editor.h 2020-09-03 21:30:46 UTC (rev 266546)
+++ branches/safari-610-branch/Source/WebCore/editing/Editor.h 2020-09-03 21:30:50 UTC (rev 266547)
@@ -336,7 +336,7 @@
#if PLATFORM(IOS_FAMILY)
NO_RETURN_DUE_TO_ASSERT
#endif
- void changeBackToReplacedString(const String& replacedString);
+ WEBCORE_EXPORT void changeBackToReplacedString(const String& replacedString);
#if !PLATFORM(IOS_FAMILY)
WEBCORE_EXPORT void advanceToNextMisspelling(bool startBeforeSelection = false);
Modified: branches/safari-610-branch/Source/WebCore/editing/TextCheckingHelper.cpp (266546 => 266547)
--- branches/safari-610-branch/Source/WebCore/editing/TextCheckingHelper.cpp 2020-09-03 21:30:46 UTC (rev 266546)
+++ branches/safari-610-branch/Source/WebCore/editing/TextCheckingHelper.cpp 2020-09-03 21:30:50 UTC (rev 266547)
@@ -106,10 +106,11 @@
static SimpleRange expandToParagraphBoundary(const SimpleRange& range)
{
- return {
- *makeBoundaryPoint(startOfParagraph(createLegacyEditingPosition(range.start))),
- *makeBoundaryPoint(endOfParagraph(createLegacyEditingPosition(range.end)))
- };
+ auto start = makeBoundaryPoint(startOfParagraph(createLegacyEditingPosition(range.start)));
+ auto end = makeBoundaryPoint(endOfParagraph(createLegacyEditingPosition(range.end)));
+ if (!start || !end)
+ return range;
+ return { *start, *end };
}
TextCheckingParagraph::TextCheckingParagraph(const SimpleRange& range)
Modified: branches/safari-610-branch/Source/WebCore/testing/Internals.cpp (266546 => 266547)
--- branches/safari-610-branch/Source/WebCore/testing/Internals.cpp 2020-09-03 21:30:46 UTC (rev 266546)
+++ branches/safari-610-branch/Source/WebCore/testing/Internals.cpp 2020-09-03 21:30:50 UTC (rev 266547)
@@ -2557,6 +2557,12 @@
frame->editor().changeSelectionListType();
}
+void Internals::changeBackToReplacedString(const String& replacedString)
+{
+ if (auto frame = makeRefPtr(this->frame()))
+ frame->editor().changeBackToReplacedString(replacedString);
+}
+
bool Internals::isOverwriteModeEnabled()
{
Document* document = contextDocument();
Modified: branches/safari-610-branch/Source/WebCore/testing/Internals.h (266546 => 266547)
--- branches/safari-610-branch/Source/WebCore/testing/Internals.h 2020-09-03 21:30:46 UTC (rev 266546)
+++ branches/safari-610-branch/Source/WebCore/testing/Internals.h 2020-09-03 21:30:50 UTC (rev 266547)
@@ -370,6 +370,7 @@
void handleAcceptedCandidate(const String& candidate, unsigned location, unsigned length);
void changeSelectionListType();
+ void changeBackToReplacedString(const String& replacedString);
bool isOverwriteModeEnabled();
void toggleOverwriteModeEnabled();
Modified: branches/safari-610-branch/Source/WebCore/testing/Internals.idl (266546 => 266547)
--- branches/safari-610-branch/Source/WebCore/testing/Internals.idl 2020-09-03 21:30:46 UTC (rev 266546)
+++ branches/safari-610-branch/Source/WebCore/testing/Internals.idl 2020-09-03 21:30:50 UTC (rev 266547)
@@ -437,6 +437,7 @@
void handleAcceptedCandidate(DOMString candidate, unsigned long location, unsigned long length);
void changeSelectionListType();
+ void changeBackToReplacedString(DOMString replacedString);
boolean isOverwriteModeEnabled();
void toggleOverwriteModeEnabled();
