Bug 211534 - Nullptr crash in InsertListCommand::doApply with user-select:none elements
Summary: Nullptr crash in InsertListCommand::doApply with user-select:none elements
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-05-06 14:47 PDT by Jack
Modified: 2020-05-07 15:45 PDT (History)
7 users (show)

See Also:


Attachments
Patch (4.21 KB, patch)
2020-05-06 15:00 PDT, Jack
no flags Details | Formatted Diff | Diff
Patch for landing (4.89 KB, patch)
2020-05-06 15:22 PDT, Jack
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jack 2020-05-06 14:47:35 PDT
<rdar:///62898521>

0   com.apple.WebCore             	0x00000001072190a2 WebCore::InsertListCommand::doApply() + 5154
1   com.apple.WebCore             	0x0000000105f23fcd WebCore::CompositeEditCommand::apply() + 397
2   com.apple.WebCore             	0x000000010720ea6d WebCore::executeInsertOrderedList(WebCore::Frame&, WebCore::Event*, WebCore::EditorCommandSource, WTF::String const&) + 109
3   com.apple.WebCore             	0x0000000105f8b7b1 WebCore::Document::execCommand(WTF::String const&, bool, WTF::String const&) + 81
4   com.apple.WebCore             	0x000000010638323a WebCore::jsDocumentPrototypeFunctionExecCommand(JSC::JSGlobalObject*, JSC::CallFrame*) + 426
Comment 1 Jack 2020-05-06 14:48:10 PDT
Root cause: In function InsertListCommand::doApply, startOfLastParagraph is an empty position, and we deref the anchorNode() in the function.

In this test case, we are inserting list in BODY till CANVAS and try to find individual paragraph to listify. However, because none of the elements meet the requirement for being end paragraph due to being non-editable or userSelect:none, startOfLastParagraph becomes an empty position.

Test case:
<style>
span { -webkit-user-select: all; }
</style>
<body id=body contentEditable="true"><span><a draggable="true">a</a><canvas id=canvas></canvas></span>
<script>
    body.appendChild(canvas);
    document.execCommand("selectAll", false);
    document.execCommand("insertOrderedList", false);
</script>

Node tree:
BODY	0x60c0001030c0 (renderer 0x61200008cd40) 
	SPAN	0x60c000103180 (renderer 0x61100022ca80) 
		A	0x60e000091ec0 (renderer 0x61100022cbc0) 
			#text	0x608000141120 "a"
	#text	0x6080001411a0 "\n"
	SCRIPT	0x610000051440 (renderer 0x0) 
		#text	0x608000141220 "\n    if (window.testRunner)\n        testRunner.dumpAsText();\n\n    body.appendChild(canvas);\n    document.execCommand("selectAll", false);\n    document.execCommand("insertOrderedList", false);\n    document.body.innerText = "Tests inserting list at the end of a table. The test passes if WebKit doesn't crash or hit an ssertion.";\n"
*	CANVAS	0x61200003e2c0 (renderer 0x61100022cd00)
Comment 2 Jack 2020-05-06 15:00:10 PDT
Created attachment 398668 [details]
Patch
Comment 3 Geoffrey Garen 2020-05-06 15:11:50 PDT
Comment on attachment 398668 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=398668&action=review

r=me

> Source/WebCore/editing/InsertListCommand.cpp:143
> +            if (!startOfLastParagraph.isNull() && startOfParagraph(startOfSelection, CanSkipOverEditingBoundary) != startOfLastParagraph) {

isNotNull() is probably better here than !isNull().
Comment 4 Jack 2020-05-06 15:22:39 PDT
Created attachment 398674 [details]
Patch for landing
Comment 5 EWS 2020-05-06 15:55:34 PDT
Committed r261255: <https://trac.webkit.org/changeset/261255>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 398674 [details].