Bug 212723 – Nullptr crash in DeleteSelectionCommand::doApply() when ending position is disconnected.

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Jack]

<rdar://63866653>

    #0 0x3ad2f71a1 in WebCore::Node::getFlag(WebCore::Node::NodeFlags) const (Safari_ASAN_262398_beabde7bd2de240129119de5a8d837355ac235e8.app/Contents/Frameworks/WebCore.framework/Versions/A/WebCore:x86_64+0x1ba1a1)
    #1 0x3b073459d in WebCore::canHaveChildrenForEditing(WebCore::Node const&) (Safari_ASAN_262398_beabde7bd2de240129119de5a8d837355ac235e8.app/Contents/Frameworks/WebCore.framework/Versions/A/WebCore:x86_64+0x35f759d)
    #2 0x3b06f5016 in WebCore::CompositeEditCommand::insertNodeAt(WTF::Ref<WebCore::Node, WTF::DumbPtrTraits<WebCore::Node> >&&, WebCore::Position const&) (Safari_ASAN_262398_beabde7bd2de240129119de5a8d837355ac235e8.app/Contents/Frameworks/WebCore.framework/Versions/A/WebCore:x86_64+0x35b8016)
    #3 0x3b0737b65 in WebCore::DeleteSelectionCommand::doApply() (Safari_ASAN_262398_beabde7bd2de240129119de5a8d837355ac235e8.app/Contents/Frameworks/WebCore.framework/Versions/A/WebCore:x86_64+0x35fab65)
    #4 0x3b070ee6a in WebCore::CompositeEditCommand::applyCommandToComposite(WTF::Ref<WebCore::EditCommand, WTF::DumbPtrTraits<WebCore::EditCommand> >&&) (Safari_ASAN_262398_beabde7bd2de240129119de5a8d837355ac235e8.app/Contents/Frameworks/WebCore.framework/Versions/A/WebCore:x86_64+0x35d1e6a)
    #5 0x3b0711f5a in WebCore::CompositeEditCommand::deleteSelection(WebCore::VisibleSelection const&, bool, bool, bool, bool, bool) (Safari_ASAN_262398_beabde7bd2de240129119de5a8d837355ac235e8.app/Contents/Frameworks/WebCore.framework/Versions/A/WebCore:x86_64+0x35d4f5a)
    #6 0x3b0827499 in WebCore::TypingCommand::deleteKeyPressed(WebCore::TextGranularity, bool) (Safari_ASAN_262398_beabde7bd2de240129119de5a8d837355ac235e8.app/Contents/Frameworks/WebCore.framework/Versions/A/WebCore:x86_64+0x36ea499)
    #7 0x3b06f2746 in WebCore::CompositeEditCommand::apply() (Safari_ASAN_262398_beabde7bd2de240129119de5a8d837355ac235e8.app/Contents/Frameworks/WebCore.framework/Versions/A/WebCore:x86_64+0x35b5746)
    #8 0x3b0826b44 in WebCore::TypingCommand::deleteKeyPressed(WebCore::Document&, unsigned int, WebCore::TextGranularity) (Safari_ASAN_262398_beabde7bd2de240129119de5a8d837355ac235e8.app/Contents/Frameworks/WebCore.framework/Versions/A/WebCore:x86_64+0x36e9b44)
    #9 0x3b07ae4de in WebCore::executeDelete(WebCore::Frame&, WebCore::Event*, WebCore::EditorCommandSource, WTF::String const&) (Safari_ASAN_262398_beabde7bd2de240129119de5a8d837355ac235e8.app/Contents/Frameworks/WebCore.framework/Versions/A/WebCore:x86_64+0x36714de)
    #10 0x3b0416d13 in WebCore::Document::execCommand(WTF::String const&, bool, WTF::String const&) (Safari_ASAN_262398_beabde7bd2de240129119de5a8d837355ac235e8.app/Contents/Frameworks/WebCore.framework/Versions/A/WebCore:x86_64+0x32d9d13)
    #11 0x3adc0e061 in WebCore::jsDocumentPrototypeFunctionExecCommandBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSDocument*, JSC::ThrowScope&) (Safari_ASAN_262398_beabde7bd2de240129119de5a8d837355ac235e8.app/Contents/Frameworks/WebCore.framework/Versions/A/WebCore:x86_64+0xad1061)
    #12 0x3adabb910 in long long WebCore::IDLOperation<WebCore::JSDocument>::call<&(WebCore::jsDocumentPrototypeFunctionExecCommandBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSDocument*, JSC::ThrowScope&)), (WebCore::CastedThisErrorBehavior)0>(JSC::JSGlobalObject&, JSC::CallFrame&, char const*) (Safari_ASAN_262398_beabde7bd2de240129119de5a8d837355ac235e8.app/Contents/Frameworks/WebCore.framework/Versions/A/WebCore:x86_64+0x97e910)
    #13 0x4f794b401177

[Jack]

This is a similar issue as <rdar://62993645> (https://bugs.webkit.org/show_bug.cgi?id=211793).

After deleting the selected elements, we try to merge the paragraphs and change the ending position to the element that is before the deleted range. However, removeNodeAndPruneAncestors is called which removes ending position from the node tree as well. Later we try to insert a BR at the parent of ending position and the code crashes since ending position is parentless now.

Before removeNodeAndPruneAncestors(), only #text is supposed to be removed, and ending position should be moved to DL.
BODY	0x11743e920 (renderer 0x11743d7f0) 
	DIV	0x11743e9b0 (renderer 0x11743ee10) 
		DL	0x11743ea40 (renderer 0x11743ef40) 
			#text	0x11743ead0 "\n"
			DT	0x11743eb30 (renderer 0x11743f070) 
*				#text	0x11743ebc0 "a"
		#text	0x11743ec20 "\n"
		SCRIPT	0x11743ec80 (renderer 0x0) 
			#text	0x11743ed50 "\n    onload = function run() {\n        window.getSelection().setPosition(dt);\n        document.execCommand("delete", false);\n    }\n"
		#text	0x11743edb0 "\n"
legacy, offset, offset:0

After removeNodeAndPruneAncestors(), ending position (DL) becomes dangling.
*DL	0x11743ea40 (renderer 0x0) 
	#text	0x11743ead0 "\n"
	DT	0x11743eb30 (renderer 0x0)

[Jack]

In this test case, removeNodeAndPruneAncestors is called in a different call stack comparing to that in 211793.

  * frame #0: 0x00000001209d6d96 WebCore`WebCore::CompositeEditCommand::removeNodeAndPruneAncestors(this=0x000000013b1e25e8, node=0x0000000135b9f450) at CompositeEditCommand.cpp:611:18
    frame #1: 0x00000001209f5874 WebCore`WebCore::DeleteSelectionCommand::mergeParagraphs(this=0x000000013b1e25e8) at DeleteSelectionCommand.cpp:731:13
    frame #2: 0x00000001209f7503 WebCore`WebCore::DeleteSelectionCommand::doApply(this=0x000000013b1e25e8) at DeleteSelectionCommand.cpp:939:5
    frame #3: 0x00000001209d5bff WebCore`WebCore::CompositeEditCommand::applyCommandToComposite(this=0x00000001393e7770, command=0x00007ffeeab0f218) at CompositeEditCommand.cpp:463:14
    frame #4: 0x00000001209d858b WebCore`WebCore::CompositeEditCommand::deleteSelection(this=0x00000001393e7770, selection=0x00007ffeeab0f818, smartDelete=false, mergeBlocksAfterDelete=true, replace=false, expandForSpecialElements=false, sanitizeMarkup=true) at CompositeEditCommand.cpp:835:9
    frame #5: 0x0000000120a9629d WebCore`WebCore::TypingCommand::deleteKeyPressed(this=0x00000001393e7770, granularity=CharacterGranularity, shouldAddToKillRing=false) at TypingCommand.cpp:748:27
    frame #6: 0x0000000120a982c8 WebCore`WebCore::TypingCommand::doApply(this=0x00000001393e7770) at TypingCommand.cpp:364:9
    frame #7: 0x00000001209c2ab5 WebCore`WebCore::CompositeEditCommand::apply(this=0x00000001393e7770) at CompositeEditCommand.cpp:372:9
    frame #8: 0x0000000120a951cc WebCore`WebCore::TypingCommand::deleteKeyPressed(document={ origin = file://, url = file:///Users/jacklee/browser2/min-63866653.html, inMainFrame = 1, backForwardCacheState = NotInBackForwardCache }, options=0, granularity=CharacterGranularity) at TypingCommand.cpp:193:86
    frame #9: 0x0000000120a42774 WebCore`WebCore::executeDelete(frame={ origin = file://, url = file:///Users/jacklee/browser2/min-63866653.html, isMainFrame = 1, backForwardCacheState = NotInBackForwardCache }, (null)=0x0000000000000000, source=CommandFromDOM, (null)={ length = 0, contents = '' }) at EditorCommand.cpp:298:9
    frame #10: 0x0000000120a1db5b WebCore`WebCore::Editor::Command::execute(this=0x00007ffeeab0fa08, parameter={ length = 0, contents = '' }, triggeringEvent=0x0000000000000000) const at EditorCommand.cpp:1876:12
    frame #11: 0x0000000120744385 WebCore`WebCore::Document::execCommand(this={ origin = file://, url = file:///Users/jacklee/browser2/min-63866653.html, inMainFrame = 1, backForwardCacheState = NotInBackForwardCache }, commandName={ length = 6, contents = 'delete' }, userInterface=false, value={ length = 0, contents = '' }) at Document.cpp:5566:54
    frame #12: 0x000000011e845bb4 WebCore`WebCore::jsDocumentPrototypeFunctionExecCommandBody(lexicalGlobalObject=0x0000000139df8468, callFrame=0x00007ffeeab0fca0, castedThis=0x000000013932cc38, throwScope=0x00007ffeeab0fc18) at JSDocument.cpp:6271:57

[Jack]

Created attachment 400985 [details]
Patch

[Jack]

In this case removeNodeAndPruneAncestors() is called in mergeParagraphs() because after deletion the merging position (DL) is collapsed (node tree below), so a BR is inserted and the merge position is moved to BR.

Collapsed at merging position:
BODY	0x111e7a920 (renderer 0x111e797f0) 
	DIV	0x111e7a9b0 (renderer 0x111e7adb0) 
*		DL	0x111e7aa40 (renderer 0x111e7aee0) 
			#text	0x111e7aad0 "\n"
			DT	0x111e7ab30 (renderer 0x111e7b010) 
		#text	0x111e7ac20 "\n"
		SCRIPT	0x111e7ac80 (renderer 0x0) 
			#text	0x111e7ad50 "\n    if (window.testRunner)\n        testRunner.dumpAsText();\n\n    window.getSelection().setPosition(dt);\n    document.execCommand("delete", false);\n    document.body.innerText = "Tests deleting text in description list. The test passes if WebKit doesn't crash or hit an ssertion.";\n"
after, offset:0

BR inserted:
BODY	0x111e7a920 (renderer 0x111e797f0) 
	DIV	0x111e7a9b0 (renderer 0x111e7adb0) 
		DL	0x111e7aa40 (renderer 0x111e7aee0) 
			#text	0x111e7aad0 "\n"
			DT	0x111e7ab30 (renderer 0x111e7b010) 
*				BR	0x111e7b3f0 (renderer 0x111e7b480) 
		#text	0x111e7ac20 "\n"
		SCRIPT	0x111e7ac80 (renderer 0x0) 
			#text	0x111e7ad50 "\n    if (window.testRunner)\n        testRunner.dumpAsText();\n\n    window.getSelection().setPosition(dt);\n    document.execCommand("delete", false);\n    document.body.innerText = "Tests deleting text in description list. The test passes if WebKit doesn't crash or hit an ssertion.";\n"
before, offset:0

Later we find that the merging position is not at right side of the bounding box, so removeNodeAndPruneAncestors() is called to remove BR and set the ending position back to start of paragraph.

[Geoffrey Garen]

Comment on attachment 400985 [details]
Patch

r=me

[EWS]

Committed r262593: <https://trac.webkit.org/changeset/262593>

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