Bug 67762

Summary: Crashes in WebCore::ReplaceSelectionCommand::doApply
Product: WebKit Reporter: Shinya Kawanaka <shinyak>
Component: HTML EditingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: rniwa, shinyak, tkent, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 67668    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Shinya Kawanaka 2011-09-07 23:40:31 PDT
https://bugs.webkit.org/show_bug.cgi?id=67668

testcase1::
<feSpotLight><sub id="div" contenteditable="true"><script>
var sel = window.getSelection();

sel.setPosition(div, 0);
document.execCommand("InsertHTML", false, "<dl>");
</script>
Comment 1 Shinya Kawanaka 2011-09-07 23:51:22 PDT
Created attachment 106700 [details]
Patch
Comment 2 Kent Tamura 2011-09-07 23:54:06 PDT
Comment on attachment 106700 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=106700&action=review

> LayoutTests/ChangeLog:11
> +        * editing/inserting/insert-replaceselection-crash-expected.txt: Added.
> +        * editing/inserting/insert-replaceselection-crash.html: Added.

It's better, but we had better describe a test case, rather than a crash location.
How about insert-without-enclosing-block.html?

> Source/WebCore/ChangeLog:10
> +        Test: editing/inserting/insert-67668-crash.html

Need to update.
Comment 3 Ryosuke Niwa 2011-09-07 23:56:36 PDT
Comment on attachment 106700 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=106700&action=review

> LayoutTests/editing/inserting/insert-replaceselection-crash.html:7
> +var sel = window.getSelection();
> +
> +sel.setPosition(div, 0);

It seems redundant to declare sel.
Comment 4 Shinya Kawanaka 2011-09-08 00:09:09 PDT
Created attachment 106703 [details]
Patch
Comment 5 Shinya Kawanaka 2011-09-08 00:09:51 PDT
Reflected the above comments.
Comment 6 Ryosuke Niwa 2011-09-08 00:11:20 PDT
Comment on attachment 106703 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=106703&action=review

> LayoutTests/ChangeLog:11
> +        * editing/inserting/insert-replaceselection-crash-expected.txt: Added.
> +        * editing/inserting/insert-replaceselection-crash.html: Added.

You should rename the test as tkent suggested.
Comment 7 Shinya Kawanaka 2011-09-08 00:12:59 PDT
(In reply to comment #6)
> (From update of attachment 106703 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=106703&action=review
> 
> > LayoutTests/ChangeLog:11
> > +        * editing/inserting/insert-replaceselection-crash-expected.txt: Added.
> > +        * editing/inserting/insert-replaceselection-crash.html: Added.
> 
> You should rename the test as tkent suggested.

Oops, sorry. I'll fix them soon.
Comment 8 Shinya Kawanaka 2011-09-08 00:43:35 PDT
Created attachment 106706 [details]
Patch
Comment 9 WebKit Review Bot 2011-09-08 13:49:30 PDT
Comment on attachment 106706 [details]
Patch

Clearing flags on attachment: 106706

Committed r94793: <http://trac.webkit.org/changeset/94793>
Comment 10 WebKit Review Bot 2011-09-08 13:49:34 PDT
All reviewed patches have been landed.  Closing bug.