<rdar://62982161> 0 com.apple.WebCore 0x00000001056e2e19 WebCore::InsertParagraphSeparatorCommand::doApply() + 9977 1 com.apple.WebCore 0x00000001056764ff WebCore::CompositeEditCommand::applyCommandToComposite(WTF::Ref<WebCore::EditCommand, WTF::DumbPtrTraits<WebCore::EditCommand> >&&) + 79 2 com.apple.WebCore 0x00000001056f36ec WebCore::ReplaceSelectionCommand::doApply() + 8076 3 com.apple.WebCore 0x00000001043e38fd WebCore::CompositeEditCommand::apply() + 397 4 com.apple.WebCore 0x00000001056d4f6a WebCore::executeInsertFragment(WebCore::Frame&, WTF::Ref<WebCore::DocumentFragment, WTF::DumbPtrTraits<WebCore::DocumentFragment> >&&) + 74 5 com.apple.WebCore 0x00000001056d50a4 WebCore::executeInsertNode(WebCore::Frame&, WTF::Ref<WebCore::Node, WTF::DumbPtrTraits<WebCore::Node> >&&) + 180 6 com.apple.WebCore 0x00000001056d05ba WebCore::executeInsertImage(WebCore::Frame&, WebCore::Event*, WebCore::EditorCommandSource, WTF::String const&) + 74 7 com.apple.WebCore 0x000000010444a5f1 WebCore::Document::execCommand(WTF::String const&, bool, WTF::String const&) + 81 8 com.apple.WebCore 0x00000001048431ca WebCore::jsDocumentPrototypeFunctionExecCommand(JSC::JSGlobalObject*, JSC::CallFrame*) + 426
Below is the test case. Thanks to Alan for minimizing it! <video><span id=span_copy><input id=input></input></span></video><button></button><audio><hr id=hr1 contentEditable="true"><hr id=hr2></audio><textarea></textarea> <script> if (window.testRunner) testRunner.dumpAsText(); hr1.appendChild(span_copy); input.setSelectionRange(-1,67); hr2.appendChild(span_copy); document.execCommand("insertImage", "#foo"); document.body.innerText = "Tests inserting list at the end of a table. The test passes if WebKit doesn't crash or hit an ssertion."; </script>
Root cause: 1. In this test case, we are inserting a image and needs a paragraph separator at the selected position: hr1. 2. In function InsertParagraphSeparatorCommand::doApply, hr1 fails the check for being candidate at both its upstream and downstream positions. 3. However, we continute to assume hr1is a valid candidate and use it to as the insertaion positiopn for the separator. 4. Since the visible position of hr1 is start of paragraph, we need to insert a br at container of hr1. 5. The container node of hr1 is audio, and it is uneditable, so the insertion fails, leaving the br dangling. 6. Next we find the new insertion position based on the dangling br, so the position becomes empty. 7. Later the null anchor node in empty position is dereferenced and the code crashes. EndSelection() is hr1: BODY 0x28b06b920 (renderer 0x28b0697f0) VIDEO 0x28b0b8010 (renderer 0x28b0b9d00) BUTTON 0x28b0b8db0 (renderer 0x28b0b9e60) AUDIO 0x28b0b8e80 (renderer 0x0) SE HR 0x28b0b97b0 (renderer 0x0) HR 0x28b0b9840 (renderer 0x0) SPAN 0x28b0b8a70 (renderer 0x0) INPUT 0x28b0b8b00 (renderer 0x0) #document-fragment 0x28b0b8c10 (renderer 0x0) (needs style recalc) (child needs style recalc) DIV 0x28b0b8d20 (renderer 0x0) (needs style recalc) TEXTAREA 0x28b0b98d0 (renderer 0x28b0ba000) #document-fragment 0x28b0b99d0 (renderer 0x0) (needs style recalc) (child needs style recalc) DIV 0x28b0b9ae0 (renderer 0x28b0ba130) #text 0x28b0b9b70 "\n" SCRIPT 0x28b0b9bd0 (renderer 0x0) #text 0x28b0b9ca0 "\n if (window.testRunner)\n testRunner.dumpAsText();\n hr1.appendChild(span_copy);\n input.setSelectionRange(-1,67);\n hr2.appendChild(span_copy);\n document.execCommand("insertImage", "#foo");\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" start: offset, offset:0 end: offset, offset:0 The container node is the parent of hr1, audio. BODY 0x28b06b920 (renderer 0x28b0697f0) VIDEO 0x28b0b8010 (renderer 0x28b0b9d00) BUTTON 0x28b0b8db0 (renderer 0x28b0b9e60) AUDIO 0x28b0b8e80 (renderer 0x0) * HR 0x28b0b97b0 (renderer 0x0) HR 0x28b0b9840 (renderer 0x0) SPAN 0x28b0b8a70 (renderer 0x0) INPUT 0x28b0b8b00 (renderer 0x0) #document-fragment 0x28b0b8c10 (renderer 0x0) (needs style recalc) (child needs style recalc) DIV 0x28b0b8d20 (renderer 0x0) (needs style recalc) TEXTAREA 0x28b0b98d0 (renderer 0x28b0ba000) #document-fragment 0x28b0b99d0 (renderer 0x0) (needs style recalc) (child needs style recalc) DIV 0x28b0b9ae0 (renderer 0x28b0ba130) #text 0x28b0b9b70 "\n" SCRIPT 0x28b0b9bd0 (renderer 0x0) #text 0x28b0b9ca0 "\n if (window.testRunner)\n testRunner.dumpAsText();\n hr1.appendChild(span_copy);\n input.setSelectionRange(-1,67);\n hr2.appendChild(span_copy);\n document.execCommand("insertImage", "#foo");\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" before, offset:0
Created attachment 399309 [details] Patch
Created attachment 399310 [details] Patch
Comment on attachment 399310 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=399310&action=review r=me > Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp:303 > + // FIXME: <http://webkit.org/b/211864> insertionPosition is not a candidate. Needs to find an editable canonical candidate. The meaning here is a little jumbled. I think we should say something like "FIXME: <http://webkit.org/b/211864>: If insertionPosition is not editable, we should compute a position that is."
Comment on attachment 399310 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=399310&action=review > Source/WebCore/ChangeLog:9 > + Check for uneditable insertion position and return. This part of the changelog should answer the question of "why" not "what".
Thanks! I will add a short explanation. (In reply to zalan from comment #6) > Comment on attachment 399310 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=399310&action=review > > > Source/WebCore/ChangeLog:9 > > + Check for uneditable insertion position and return. > > This part of the changelog should answer the question of "why" not "what".
Created attachment 399318 [details] Patch for landing
Committed r261666: <https://trac.webkit.org/changeset/261666> All reviewed patches have been landed. Closing bug and clearing flags on attachment 399318 [details].