Bug 32390

Summary: Crash in WebCore::SplitElementCommand::SplitElementCommand ReadAV@NULL (3d3ebb81789addf3fcdd28c39a72b244)
Product: WebKit Reporter: Berend-Jan Wever <skylined>
Component: HTML EditingAssignee: Tony Chang <tony>
Status: RESOLVED FIXED    
Severity: Normal CC: adele, commit-queue, enrica, eric, oliver, rniwa, tony
Priority: P1    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows Vista   
URL: http://skypher.com/SkyLined/Repro/WebKit/Bug%2032390%20-%20WebCore..SplitElementCommand..SplitElementCommand%20ReadAV@NULL%20(3d3ebb81789addf3fcdd28c39a72b244)/repro.html
Attachments:
Description Flags
Repro
none
Patch
none
Patch none

Description Berend-Jan Wever 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>
Comment 1 Berend-Jan Wever 2009-12-10 12:30:49 PST
Online repro
Comment 2 Berend-Jan Wever 2009-12-10 12:40:19 PST
Fix online repro link
Comment 3 Tony Chang 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.
Comment 4 Tony Chang 2010-01-12 23:17:47 PST
Created attachment 46427 [details]
Patch
Comment 5 Tony Chang 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.
Comment 6 Eric Seidel (no email) 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.
Comment 7 Tony Chang 2010-01-26 17:25:06 PST
Created attachment 47473 [details]
Patch
Comment 8 Tony Chang 2010-01-26 17:25:39 PST
Comment on attachment 47473 [details]
Patch

just adding the fixme
Comment 9 Eric Seidel (no email) 2010-01-26 17:51:52 PST
Comment on attachment 47473 [details]
Patch

LGTM.
Comment 10 WebKit Commit Bot 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>
Comment 11 WebKit Commit Bot 2010-01-27 05:51:31 PST
All reviewed patches have been landed.  Closing bug.