Summary: | Crash inside Editor::styleForSelectionStart | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ryosuke Niwa <rniwa> | ||||
Component: | HTML Editing | Assignee: | Ryosuke Niwa <rniwa> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | cdumez, darin, enrica, wenson_hsieh | ||||
Priority: | P2 | ||||||
Version: | Safari 10 | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Ryosuke Niwa
2017-01-04 19:15:59 PST
Created attachment 298060 [details]
Fixes the crash
Comment on attachment 298060 [details] Fixes the crash View in context: https://bugs.webkit.org/attachment.cgi?id=298060&action=review > Source/WebCore/editing/cocoa/EditorCocoa.mm:79 > + RefPtr<Node> positionNode = position.deprecatedNode(); Why not Node* (or auto or auto*) instead of RefPtr<Node>? Doesn’t seem necessary to churn the reference count here. (In reply to comment #2) > Comment on attachment 298060 [details] > Fixes the crash > > View in context: > https://bugs.webkit.org/attachment.cgi?id=298060&action=review > > > Source/WebCore/editing/cocoa/EditorCocoa.mm:79 > > + RefPtr<Node> positionNode = position.deprecatedNode(); > > Why not Node* (or auto or auto*) instead of RefPtr<Node>? Doesn’t seem > necessary to churn the reference count here. I'd use auto here. I think using a raw pointer in editing code is a good idea because if someone starts using that raw pointe after calling appendChild, that could turn into a security vulnerability. And the risk and the cost of having to think about all those implications everywhere in editing is worth avoiding one ref churn. Committed r210376: <http://trac.webkit.org/changeset/210376> |