Bug 45218 - Move functions from Frame to Editor as planned
Summary: Move functions from Frame to Editor as planned
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Darin Adler
URL:
Keywords:
: 23430 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-09-03 17:56 PDT by Darin Adler
Modified: 2012-05-30 00:54 PDT (History)
6 users (show)

See Also:

Attachments
Patch (109.55 KB, patch)
2010-09-08 17:36 PDT, Darin Adler 		no flags Details | Formatted Diff | Diff
Patch (114.52 KB, patch)
2010-09-09 10:52 PDT, Darin Adler 		abarth: review+
abarth: commit-queue-
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Adler 2010-09-03 17:56:19 PDT 
Move functions from Frame to Editor as planned
Comment 1 Darin Adler 2010-09-08 17:36:01 PDT 
Created attachment 66972 [details]
Patch
Comment 2 Early Warning System Bot 2010-09-08 17:57:16 PDT 
Attachment 66972 [details] did not build on qt:
Build output: http://queues.webkit.org/results/3949295
Comment 3 WebKit Review Bot 2010-09-08 21:08:40 PDT 
Attachment 66972 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/3979090
Comment 4 WebKit Review Bot 2010-09-08 21:47:14 PDT 
Attachment 66972 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/3919332
Comment 5 WebKit Review Bot 2010-09-09 07:52:17 PDT 
Attachment 66972 [details] did not build on win:
Build output: http://queues.webkit.org/results/3985048
Comment 6 Darin Adler 2010-09-09 10:52:42 PDT 
Created attachment 67059 [details]
Patch
Comment 7 Adam Barth 2010-09-09 12:35:52 PDT 
Comment on attachment 67059 [details]
Patch

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

> WebCore/editing/Editor.cpp:3366
> +    styleElement->appendChild(m_frame->document()->createEditingTextNode(""), ec);
> +    ASSERT(!ec);
This is a security vulnerability.  appendChild fires DOM mutation events, which run JavaScript, which can make |node| point off into unallocated memory.  Please RefPtr node.

> WebCore/editing/Editor.cpp:3394
> +    Node* shadowTreeRoot = selection.shadowTreeRootNode();
I bet this needs to be RefPtr for the same reason.
Comment 8 Darin Adler 2010-09-09 14:16:48 PDT 
(In reply to comment #7)
> (From update of attachment 67059 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=67059&action=prettypatch
> 
> > WebCore/editing/Editor.cpp:3366
> > +    styleElement->appendChild(m_frame->document()->createEditingTextNode(""), ec);
> > +    ASSERT(!ec);
> This is a security vulnerability.  appendChild fires DOM mutation events, which run JavaScript, which can make |node| point off into unallocated memory.  Please RefPtr node.

Glad you spotted it. But the node here came from position.node(); Position already holds the node in a RefPtr. I’ll change the code to use position.node() and eliminate the local variable for clarity on this point.

> > WebCore/editing/Editor.cpp:3394
> > +    Node* shadowTreeRoot = selection.shadowTreeRootNode();
> I bet this needs to be RefPtr for the same reason.

I put this in a RefPtr.
Comment 9 Darin Adler 2010-09-09 16:09:16 PDT 
Committed r67122: <http://trac.webkit.org/changeset/67122>
Comment 10 Ryosuke Niwa 2012-05-30 00:54:10 PDT 
*** Bug 23430 has been marked as a duplicate of this bug. ***