Created attachment 62732 [details] Repro - WebCore::AppendNodeCommand::AppendNodeCommand ReadAV@NULL (ae8a44eb1a4d3115a1236246b62cf2d1) <html> <head> <script> document.designMode="on"; document.writeln('x<isIndex/><x><object>x</object>'); selection=window.getSelection(); selection.selectAllChildren(document); document.execCommand("indent"); </script> </head> <body onload="go()"> </body> </html> The above causes a NULL pointer crash: id: WebCore::AppendNodeCommand::AppendNodeCommand ReadAV@NULL (ae8a44eb1a4d3115a1236246b62cf2d1) description: Attempt to read from NULL pointer (+0x14) in WebCore::AppendNodeCommand::AppendNodeCommand stack: WebCore::AppendNodeCommand::AppendNodeCommand WebCore::AppendNodeCommand::create WebCore::CompositeEditCommand::appendNode WebCore::CompositeEditCommand::cloneParagraphUnderNewElement WebCore::CompositeEditCommand::moveParagraphWithClones WebCore::IndentOutdentCommand::indentIntoBlockquote WebCore::IndentOutdentCommand::indentRegion WebCore::IndentOutdentCommand::doApply WebCore::EditCommand::apply WebCore::applyCommand WebCore::executeIndent WebCore::Editor::Command::execute WebCore::Document::execCommand ...
Created attachment 62796 [details] Crash report using Mac nightly r64156
The assertion on line 39 of AppendNodeCommand.cpp <http://trac.webkit.org/browser/trunk/WebCore/editing/AppendNodeCommand.cpp?rev=51645#L39> fails: ASSERT(m_node)
This bug seems to be caused by indentIntoBlockquote calling moveParagraphWithClones with a wrong outerBlock (i.e. outerBlock doesn't contain startOfParagraphToMove). As a result, cloneParagraphUnderNewElement tries to copy everything up to Document node and fails on appendChild.
I spent some time on this bug and I think the problem is that when there are nodes between nodeToSplitTo and start.node(), splitTreeToNode may shift the start of paragraph to inside of the nodes being duplicated. For example, when we are on the following node in the attached reduction: BODY 0x106263a00 BLOCKQUOTE 0x1062367d0 CLASS=webkit-indent-blockquote STYLE=margin: 0 0 0 40px; border: none; padding: 0px; #text 0x108280ee0 "x" FORM 0x108279320 HR 0x108291aa0 FORM 0x106287c80 LABEL 0x106287380 #text 0x11a802d80 "This is a searchable index. Enter search keywords: " INPUT 0x11a802f40 FORM 0x106284e90 HR 0x106285150 X 0x1062856e0 OBJECT 0x106285940 * #text 0x106285ab0 "x" #text 0x106285ec0 "\n" We end up copying X and Object to get: BODY 0x106263a00 BLOCKQUOTE 0x1062367d0 CLASS=webkit-indent-blockquote STYLE=margin: 0 0 0 40px; border: none; padding: 0px; #text 0x108280ee0 "x" FORM 0x108279320 HR 0x108291aa0 FORM 0x106287c80 LABEL 0x106287380 #text 0x11a802d80 "This is a searchable index. Enter search keywords: " INPUT 0x11a802f40 FORM 0x106284e90 HR 0x106285150 X 0x106284e10 @ OBJECT 0x106270fa0 $ X 0x1062856e0 OBJECT 0x106285940 * #text 0x106285ab0 "x" #text 0x106285ec0 "\n" At which point the start of paragraph is shifted to @ even though the outerBlock is still at $.
Replacing RefPtr<Node> outerBlock = (start.node() == nodeToSplitTo) ? start.node() : splitTreeToNode(start.node(), nodeToSplitTo); by + RefPtr<Node> outerBlock = start.node(); + bool different = false; + for (Node* n = start.node(); n && n != nodeToSplitTo; n = n->parentNode()) + if (visiblePositionBeforeNode(n) != visiblePositionBeforeNode(start.node())) + different = true; + if (start.node() != nodeToSplitTo && different) + outerBlock = splitTreeToNode(start.node(), nodeToSplitTo); seems to work. The idea is to avoid splitting the tree when nodes between start.node() and nodeToSplitTo belong to the current paragraph.
Created attachment 69143 [details] nastier crash This is a reduced test cause of the crash report sent by David Levin.
Could you explain why that crash is nastier? It's also a NULL pointer, so security wise it doesn't seem to matter much.
Actually, it triggers the exact same NULL pointer crash for me as the original repro.
(In reply to comment #7) > Could you explain why that crash is nastier? It's also a NULL pointer, so security wise it doesn't seem to matter much. (In reply to comment #8) > Actually, it triggers the exact same NULL pointer crash for me as the original repro. Yes, it does trigger the same crash but this one cannot be fully fixed by my suggested fix. Even though the crash ceases I still get an unexpected value.
This no longer reproduces in latest Chromium.