Closed Bug 578905 Opened 10 years ago Closed 10 years ago

nsRange methods from DOM interfaces should use NS_IMETHODIMP on their implementations

Categories

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

x86
All
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla2.0b2

People

(Reporter: craig.topper, Assigned: craig.topper)

Details

Attachments

(1 file, 1 obsolete file)

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.
Ok so after some searching on google this might actually be defined behavior, but is inconsistent with the rest of the code base.
Attached patch Patch (obsolete) — Splinter Review
Attachment #457770 - Flags: review?(bzbarsky)
Comment on attachment 457770 [details] [diff] [review]
Patch

r=bzbarsky
Attachment #457770 - Flags: review?(bzbarsky) → review+
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.
(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.
Attachment #457770 - Attachment is obsolete: true
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/95cae32d3eee
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b2
Component: DOM: Traversal-Range → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.