Closed
Bug 1284963
Opened 8 years ago
Closed 8 years ago
Make CreateBR return already_AddRefed
Categories
(Core :: DOM: Editor, defect)
Core
DOM: Editor
Tracking
()
RESOLVED
FIXED
mozilla50
Tracking | Status | |
---|---|---|
firefox50 | --- | fixed |
People
(Reporter: mccr8, Assigned: mccr8)
References
Details
Attachments
(1 file)
7.07 KB,
patch
|
masayuki
:
review+
|
Details | Diff | Splinter Review |
Bug 1266882 has demonstrated that returning a raw pointer is error-prone, so let's just revert part 5 of bug 1156062. I'll wait to land this until bug 1260651 has landed, to avoid rebasing trouble for that patch.
Assignee | ||
Comment 1•8 years ago
|
||
Returning already_AddRefed rather than a raw pointer makes it harder to make mistakes with refcounting like the one seen in bug 1266882.
Attachment #8769879 -
Flags: review?(masayuki)
Comment 2•8 years ago
|
||
Comment on attachment 8769879 [details] [diff] [review]
Make CreateBR return already_AddRefed.
>diff --git a/editor/libeditor/WSRunObject.h b/editor/libeditor/WSRunObject.h
>index a7cbeb50..be6c7c5 100644
>--- a/editor/libeditor/WSRunObject.h
>+++ b/editor/libeditor/WSRunObject.h
>@@ -210,19 +210,20 @@ public:
> static nsresult PrepareToSplitAcrossBlocks(HTMLEditor* aHTMLEditor,
> nsCOMPtr<nsINode>* aSplitNode,
> int32_t* aSplitOffset);
>
> // InsertBreak inserts a br node at {aInOutParent,aInOutOffset}
> // and makes any needed adjustments to ws around that point.
> // example of fixup: normalws after {aInOutParent,aInOutOffset}
> // needs to begin with nbsp.
>- dom::Element* InsertBreak(nsCOMPtr<nsINode>* aInOutParent,
>- int32_t* aInOutOffset,
>- nsIEditor::EDirection aSelect);
>+ already_AddRefed<mozilla::dom::Element>
nit: |mozilla::| isn't necessary. |dom::Element| should be enough.
Attachment #8769879 -
Flags: review?(masayuki) → review+
Pushed by amccreight@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3c622421db18
Make CreateBR return already_AddRefed. r=masayuki
Assignee | ||
Comment 4•8 years ago
|
||
(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #2)
> nit: |mozilla::| isn't necessary. |dom::Element| should be enough.
Fixed.
Comment 5•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in
before you can comment on or make changes to this bug.
Description
•