578905 - nsRange methods from DOM interfaces should use NS_IMETHODIMP on their implementations

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Craig Topper]

Most of the methods in nsRange that come from DOM interfaces don't use NS_IMETHODIMP on their implementations, just nsresult. Strangely, this compiles. Guess the compiler on Windows is happy to let the declaration use _stdcall without the implementation saying it. The opposite is not true for obvious reasons.

Probaby a good idea to fix this anyway in case some future compiler fixes this oddity.

Comment 1

[Craig Topper]

Ok so after some searching on google this might actually be defined behavior, but is inconsistent with the rest of the code base.

Comment 2

[Craig Topper]

Attachment: Patch

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 457770 [details] [diff] [review]
Patch

r=bzbarsky

Comment 4

[Alex Vincent [:WeirdAl]]

Comment on attachment 457770 [details] [diff] [review]
Patch

>-nsresult nsRange::CutContents(nsIDOMDocumentFragment** aFragment)
>-{ 
>+NS_IMETHODIMP
>+nsRange::CutContents(nsIDOMDocumentFragment** aFragment)
>+{

Are you sure about this one?  CutContents was an internal method I originally wrote (and smaug later took ownership of).  It's not exposed directly through the interface for nsIDOMRange or nsIDOMNSRange.

Comment 5

[Craig Topper]

(In reply to comment #4)
> Comment on attachment 457770 [details] [diff] [review]
> Patch
> 
> >-nsresult nsRange::CutContents(nsIDOMDocumentFragment** aFragment)
> >-{ 
> >+NS_IMETHODIMP
> >+nsRange::CutContents(nsIDOMDocumentFragment** aFragment)
> >+{
> 
> Are you sure about this one?  CutContents was an internal method I originally
> wrote (and smaug later took ownership of).  It's not exposed directly through
> the interface for nsIDOMRange or nsIDOMNSRange.

Oops. You're right. I'll fix the patch.

Comment 6

[Craig Topper]

Attachment: Reverted accidental change to CutContents

Comment 7

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/95cae32d3eee