RESOLVED FIXED32390
Crash in WebCore::SplitElementCommand::SplitElementCommand ReadAV@NULL (3d3ebb81789addf3fcdd28c39a72b244)
https://bugs.webkit.org/show_bug.cgi?id=32390
Summary Crash in WebCore::SplitElementCommand::SplitElementCommand ReadAV@NULL (3d3eb...
Berend-Jan Wever
Reported 2009-12-10 12:27:33 PST
Created attachment 44632 [details] Repro Id: WebCore::SplitElementCommand::SplitElementCommand ReadAV@NULL (3d3ebb81789addf3fcdd28c39a72b244) Description: Attempt to read from NULL pointer (+0x10) in WebCore::SplitElementCommand::SplitElementCommand Stack: WebCore::SplitElementCommand::SplitElementCommand WebCore::SplitElementCommand::create WebCore::CompositeEditCommand::splitTreeToNode WebCore::IndentOutdentCommand::indentIntoBlockquote WebCore::IndentOutdentCommand::indentRegion WebCore::IndentOutdentCommand::doApply WebCore::EditCommand::apply WebCore::applyCommand WebCore::executeIndent WebCore::Editor::Command::execute WebCore::Document::execCommand WebCore::DocumentInternal::execCommandCallback Repro: <BODY></BODY> <SCRIPT> document.designMode = "on"; document.execCommand("SelectAll",false,""); document.execCommand("InsertUnorderedList",""); document.execCommand("inserthorizontalrule",false, "??"); document.execCommand("indent",false,3); </SCRIPT>
Attachments
Repro (267 bytes, text/html)
2009-12-10 12:27 PST, Berend-Jan Wever
no flags
Patch (3.94 KB, patch)
2010-01-12 23:17 PST, Tony Chang
no flags
Patch (4.09 KB, patch)
2010-01-26 17:25 PST, Tony Chang
no flags
Berend-Jan Wever
Comment 1 2009-12-10 12:30:49 PST
Online repro
Berend-Jan Wever
Comment 2 2009-12-10 12:40:19 PST
Fix online repro link
Tony Chang
Comment 3 2010-01-12 23:13:11 PST
What should happen is that we have a list item with a horizontal rule in it that gets indented to be a 2 level deep list item. The call to insert should just indent the list item one level in. Instead, it doesn't try to indent the list item and gets confused and tries to put a blockquote around the HR. It doesn't try to indent the list item because enclosingBlock(Node*) in htmlediting.cpp can return the current element. The FIXME on the function says: // FIXME: Pass a position to this function. The enclosing block of [table, x] for example, should be the // block that contains the table and not the table, and this function should be the only one responsible for // knowing about these kinds of special cases. I'm not sure how passing a Position into the function will fix this. However, I can work around this at the call site and call enclosingBlock a second time if it returns the passed in Node. Patch coming that demonstrates this approach.
Tony Chang
Comment 4 2010-01-12 23:17:47 PST
Tony Chang
Comment 5 2010-01-12 23:19:16 PST
I'm not sure this is the best way to solve this. It'd be better to fix enclosingBlock(Node*), but I need some guidance on how to do that.
Eric Seidel (no email)
Comment 6 2010-01-26 14:13:28 PST
Comment on attachment 46427 [details] Patch I think the fix and test are fine. Certainly better than crashing. You should add a FIXME next to your fix saying that enclosingBlock should eventually be fixed instead.
Tony Chang
Comment 7 2010-01-26 17:25:06 PST
Tony Chang
Comment 8 2010-01-26 17:25:39 PST
Comment on attachment 47473 [details] Patch just adding the fixme
Eric Seidel (no email)
Comment 9 2010-01-26 17:51:52 PST
Comment on attachment 47473 [details] Patch LGTM.
WebKit Commit Bot
Comment 10 2010-01-27 05:51:21 PST
Comment on attachment 47473 [details] Patch Clearing flags on attachment: 47473 Committed r53927: <http://trac.webkit.org/changeset/53927>
WebKit Commit Bot
Comment 11 2010-01-27 05:51:31 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.