Bug 173085 – Crash inside InsertNodeBeforeCommand via InsertParagraphSeparatorCommand

• Component: HTML Editing
• Status: RESOLVED FIXED

[Ryosuke Niwa]

#0 0x113068d70 in WebCore::Node::treeScope() const (/Users/singi/safari/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore+0x4d70)
   #1 0x113068d38 in WebCore::Node::document() const (/Users/singi/safari/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore+0x4d38)
   #2 0x1140172d6 in WebCore::InsertNodeBeforeCommand::InsertNodeBeforeCommand(WTF::RefPtr<WebCore::Node>&&, WTF::RefPtr<WebCore::Node>&&, WebCore::ShouldAssumeContentIsAlwaysEditable, WebCore::EditAction) (/Users/singi/safari/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore+0xfb32d6)
   #3 0x1133d03af in WebCore::InsertNodeBeforeCommand::create(WTF::RefPtr<WebCore::Node>&&, WTF::RefPtr<WebCore::Node>&&, WebCore::ShouldAssumeContentIsAlwaysEditable, WebCore::EditAction) (/Users/singi/safari/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore+0x36c3af)
   #4 0x1133d0147 in WebCore::CompositeEditCommand::insertNodeBefore(WTF::PassRefPtr<WebCore::Node>, WTF::PassRefPtr<WebCore::Node>, WebCore::ShouldAssumeContentIsAlwaysEditable) (/Users/singi/safari/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore+0x36c147)
   #5 0x114019dd6 in WebCore::InsertParagraphSeparatorCommand::doApply() (/Users/singi/safari/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore+0xfb5dd6)
   #6 0x1133ced57 in WebCore::CompositeEditCommand::applyCommandToComposite(WTF::PassRefPtr<WebCore::EditCommand>) (/Users/singi/safari/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore+0x36ad57)
   #7 0x1133cfdc8 in WebCore::CompositeEditCommand::insertParagraphSeparator(bool, bool) (/Users/singi/safari/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore+0x36bdc8)
   #8 0x115789540 in WebCore::ReplaceSelectionCommand::doApply() (/Users/singi/safari/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore+0x2725540)
   #9 0x1133cdcea in WebCore::CompositeEditCommand::apply() (/Users/singi/safari/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore+0x369cea)
   #10 0x11393e06f in WebCore::executeInsertFragment(WebCore::Frame&, WTF::PassRefPtr<WebCore::DocumentFragment>) (/Users/singi/safari/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore+0x8da06f)
   #11 0x11393e350 in WebCore::executeInsertNode(WebCore::Frame&, WTF::Ref<WebCore::Node>&&) (/Users/singi/safari/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore+0x8da350)
   #12 0x113937fc1 in WebCore::executeInsertHorizontalRule(WebCore::Frame&, WebCore::Event*, WebCore::EditorCommandSource, WTF::String const&) (/Users/singi/safari/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore+0x8d3fc1)
   #13 0x113934e57 in WebCore::Editor::Command::execute(WTF::String const&, WebCore::Event*) const (/Users/singi/safari/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore+0x8d0e57)
   #14 0x1137cb329 in WebCore::Document::execCommand(WTF::String const&, bool, WTF::String const&) (/Users/singi/safari/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore+0x767329)

[Ryosuke Niwa]

Created attachment 312270 [details]
Fixes the bug

[Ryosuke Niwa]

Created attachment 312271 [details]
Updated the change log

[Ryosuke Niwa]

<rdar://problem/32575059>

[Wenson Hsieh]

Comment on attachment 312271 [details]
Updated the change log

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

> Source/WebCore/ChangeLog:5
> +

Is there a radar link we can include?

> Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp:198
> +    if (visiblePos.isNull())

Nit - Can we move this to right after we create visiblePos, so we don't have to calculateStyleBeforeInsertion() unnecessarily in the case that visiblePos.isNull()?

[Ryosuke Niwa]

(In reply to Wenson Hsieh from comment #4)
> Comment on attachment 312271 [details]
> Updated the change log
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=312271&action=review
> 
> > Source/WebCore/ChangeLog:5
> > +
> 
> Is there a radar link we can include?

Added.

> > Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp:198
> > +    if (visiblePos.isNull())
> 
> Nit - Can we move this to right after we create visiblePos, so we don't have
> to calculateStyleBeforeInsertion() unnecessarily in the case that
> visiblePos.isNull()?

Fixed.

[Ryosuke Niwa]

Committed r217958: <http://trac.webkit.org/changeset/217958>