Reproduces in google presentations. (1) goto http://docs.google.com (2) create new presentation (3) enter some text in box (4) select portion of it (5) drag/drop the text (i can get it to crash fairly reliably when dropping on itself, and when dropping outside of text box) Reproduced crash in: Safari 3.1 (mac/winxp) WebKit r35806 winxp WebKit r35844 mac
Created attachment 22910 [details] Crash on WebKit r35844 Mac OS X top of call stack: WebCore::Node::nodeIndex() const + 6 WebCore::positionBeforeNode(WebCore::Node const*) + 26 WebCore::ReplaceSelectionCommand::positionAtStartOfInsertedContent() + 35 WebCore::ReplaceSelectionCommand::doApply() + 7044 WebCore::EditCommand::apply() + 76 WebCore::DragController::concludeDrag(WebCore::DragData*, WebCore::DragDestinationAction) + 2724
Created attachment 22911 [details] Crash on WebKit r35806 WinXP top of call stack: WebCore::InsertNodeBeforeCommand::doApply+0x2a WebCore::EditCommand::apply+0xe2 WebCore::CompositeEditCommand::applyCommandToComposite+0x16 WebCore::CompositeEditCommand::insertNodeBefore+0x45 WebCore::RemoveNodePreservingChildrenCommand::doApply+0x39
<rdar://problem/5625820>
This hits the same assert as bug 19066, could be the same problem. ASSERT(isStartOfParagraph(startOfParagraphToMove)); Callstack: WebCore::CompositeEditCommand::moveParagraph WebCore::ReplaceSelectionCommand::doApply WebCore::EditCommand::apply WebCore::applyCommand
Might be related to https://bugs.webkit.org/show_bug.cgi?id=28697
Created attachment 65772 [details] Reduction I'm fairly confident that this is the problem here. Running this test in Debug, we hit an assert sooner that helps identify the problem.
The problem is occurring when replacing at the border with a span. if you have: <p><span style='background-color: red'>hello</span>world</p> and you try to replace 'wo' with '<p>Wo</p> we end up in a situation where the replaced content will be inside the span. This is not only wrong, but with some styles it crashes.
Created attachment 65775 [details] Patch
Comment on attachment 65775 [details] Patch The expected results are a little hard to read, but I get that there's not supposed to be a crash :) Looks good!
Comment on attachment 65775 [details] Patch WebCore/editing/ReplaceSelectionCommand.cpp:1000 + RefPtr<Node>placeholder = createBreakElement(document()); Need a space after the ">". WebCore/editing/ReplaceSelectionCommand.cpp:1001 + insertNodeBefore(placeholder, refNode.get()); This could be placeholder.release() to avoid a tiny bit of reference count churn. Or you could just put the createBreakElement function call right into the insertNodeBefore function call, which would also fix both of the above issues.
(In reply to comment #10) > (From update of attachment 65775 [details]) > WebCore/editing/ReplaceSelectionCommand.cpp:1000 > + RefPtr<Node>placeholder = createBreakElement(document()); > Need a space after the ">". > Sorry, this is a leftover from a previous version where I was keeping placeholder to remove it later, but it wasn't necessary. I've changed the code as you suggested. > WebCore/editing/ReplaceSelectionCommand.cpp:1001 > + insertNodeBefore(placeholder, refNode.get()); > This could be placeholder.release() to avoid a tiny bit of reference count churn. > > Or you could just put the createBreakElement function call right into the insertNodeBefore function call, which would also fix both of the above issues.
Committed revision 66344.
http://trac.webkit.org/changeset/66344 might have broken Qt Linux Release