nsIDOMHTMLMediaElement's currentTime attribute is overridden by Windows macro

RESOLVED FIXED in mozilla1.9.2a1

Status

()

defect
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: Seno.Aiko, Assigned: Seno.Aiko)

Tracking

({fixed1.9.1})

Trunk
mozilla1.9.2a1
x86
Windows Vista
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Assignee

Description

11 years ago
Posted patch undef GetCurrentTime (obsolete) — Splinter Review
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.
Attachment #349720 - Flags: superreview?(vladimir)
Attachment #349720 - Flags: review?(chris.double)
Attachment #349720 - Flags: superreview?(vladimir)
Attachment #349720 - Flags: superreview+
Attachment #349720 - Flags: review?(chris.double)
Attachment #349720 - Flags: review+
Assignee

Updated

11 years ago
Keywords: checkin-needed
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
}
Attachment #349720 - Attachment is obsolete: true
Assignee

Comment 2

11 years ago
I hope the uuid doesn't change again before this is checked in.
Assignee

Updated

11 years ago
Keywords: checkin-needed
Comment on attachment 352530 [details] [diff] [review]
same patch with the current uuid
[Checkin: Comment 3]

http://hg.mozilla.org/mozilla-central/rev/408fe87e1242
Attachment #352530 - Attachment description: same patch with the current uuid → same patch with the current uuid [Checkin: Comment 3]
NB: I'd like to get this into 1.9.1 too...
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
Pushed 49a3dae96ab3 to 1.9.1
Keywords: fixed1.9.1
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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.
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED

Updated

11 years ago
Blocks: 472799
You need to log in before you can comment on or make changes to this bug.