Closed Bug 677340 Opened 10 years ago Closed 10 years ago

Stop returning nsCOMPtr by value in various places

Categories

(Core :: DOM: Editor, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla8

People

(Reporter: Ms2ger, Assigned: Ms2ger)

Details

Attachments

(2 files)

Attached patch Patch v1Splinter Review
I believe you owe me a review
Attachment #551531 - Flags: review?(ehsan)
Flags: in-testsuite-
Comment on attachment 551531 [details] [diff] [review]
Patch v1

Review of attachment 551531 [details] [diff] [review]:
-----------------------------------------------------------------

::: editor/libeditor/html/nsHTMLEditor.cpp
@@ +941,2 @@
>    if (NS_FAILED(aNode->GetParentNode(getter_AddRefs(p))))  // no parent, ran off top of tree
> +    return NULL;

Use nsnull, please.

@@ +1061,3 @@
>            {
> +            // add range to the list if it doesn't overlap with the previous range
> +            bool addRange = true;

I'm gonna ignore you sneaking in this change, because I like it.  It just makes me sad that you didn't do the same for isNotInlineOrText.  ;-)

@@ +1115,5 @@
>  
>    nsresult rv;
>    nsCOMPtr<nsIContentIterator> iter =
>         do_CreateInstance("@mozilla.org/content/post-content-iterator;1", &rv);
> +  NS_ENSURE_SUCCESS(rv, NULL);

nsnull.

@@ +1151,5 @@
>      else
>        iter->Prev();
>    }
>    
> +  return NULL;

nsnull.
Attachment #551531 - Flags: review?(ehsan) → review+
(In reply to Ehsan Akhgari [:ehsan] from comment #2)
> Comment on attachment 551531 [details] [diff] [review]
> @@ +1061,3 @@
> >            {
> > +            // add range to the list if it doesn't overlap with the previous range
> > +            bool addRange = true;
> 
> I'm gonna ignore you sneaking in this change, because I like it.  It just
> makes me sad that you didn't do the same for isNotInlineOrText.  ;-)

I would have, but it's used as a PRBool* outparam.
http://hg.mozilla.org/mozilla-central/rev/0a8dcfddf43f
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
You need to log in before you can comment on or make changes to this bug.