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>
Online repro
Fix online repro link
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.
Created attachment 46427 [details] Patch
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.
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.
Created attachment 47473 [details] Patch
Comment on attachment 47473 [details] Patch just adding the fixme
Comment on attachment 47473 [details] Patch LGTM.
Comment on attachment 47473 [details] Patch Clearing flags on attachment: 47473 Committed r53927: <http://trac.webkit.org/changeset/53927>
All reviewed patches have been landed. Closing bug.