466432 - nsIDOMHTMLMediaElement's currentTime attribute is overridden by Windows macro

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Aiko]

Attachment: undef GetCurrentTime

During the build of nsDOMClassinfo.cpp on Windows you get this warning:

nsIDOMHTMLMediaElement.h(111) : warning C4002: too many actual parameters for macro 'GetCurrentTime'

This is the same problem as bug 328368, so I guess the same patch is in order.

Comment 1

[Serge Gautherie (:sgautherie)]

Comment on attachment 349720 [details] [diff] [review]
undef GetCurrentTime

Sorry:
{
patching file dom/public/idl/html/nsIDOMHTMLMediaElement.idl
Hunk #1 FAILED at 45
1 out of 1 hunks FAILED
}

Comment 2

[Aiko]

Attachment: same patch with the current uuid [Checkin: Comment 3]

I hope the uuid doesn't change again before this is checked in.

Comment 3

[Serge Gautherie (:sgautherie)]

Comment on attachment 352530 [details] [diff] [review]
same patch with the current uuid
[Checkin: Comment 3]

http://hg.mozilla.org/mozilla-central/rev/408fe87e1242

Comment 4

[Serge Gautherie (:sgautherie)]

NB: I'd like to get this into 1.9.1 too...

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Pushed 49a3dae96ab3 to 1.9.1

Comment 6

[Jeff Walden [:Waldo]]

This is the wrong patch: suppose someone decides they want to use one of the media IDL files in their binary extensions, xulrunner app, whatever, and they also want to use the GetCurrentTime Windows macro -- they lose.  The correct solution is to use the XPIDL binaryname annotation on the attribute, which lets you override the default C++ method name for an interface attribute or method:

http://mxr.mozilla.org/mozilla-central/source/xpcom/base/nsIConsoleMessage.idl#48

In that case the C++ method name instead became GetMessageMoz (and SetMessageMoz had the attribute not been readonly).

I think this fix is wrong, and I think this should be fixed correctly.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

It's not that bad. We have #undefs like this elsewhere and it hasn't been a problem in pratice. The fix you describe is better, sure, but we don't have to block on that. Please file a followup bug.