Bug 67762 - Crashes in WebCore::ReplaceSelectionCommand::doApply
Summary: Crashes in WebCore::ReplaceSelectionCommand::doApply
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 67668
  Show dependency treegraph
 
Reported: 2011-09-07 23:40 PDT by Shinya Kawanaka
Modified: 2011-09-08 13:49 PDT (History)
4 users (show)

See Also:


Attachments
Patch (3.60 KB, patch)
2011-09-07 23:51 PDT, Shinya Kawanaka
no flags Details | Formatted Diff | Diff
Patch (3.59 KB, patch)
2011-09-08 00:09 PDT, Shinya Kawanaka
no flags Details | Formatted Diff | Diff
Patch (3.61 KB, patch)
2011-09-08 00:43 PDT, Shinya Kawanaka
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.