362799 - warning C4244: '=' : conversion from 'PRTime' to 'PRUint32', possible loss of data

Status: RESOLVED WORKSFORME

(Core :: Widget, defect)

Description

[Mats Palmgren (inactive)]

When compiling this line:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/content/events/src/nsDOMXULCommandEvent.cpp&rev=1.7&root=/cvsroot&mark=52#41

I get this warning (Windows XP, Visual compiler):
c:/moz/firefox/mozilla/content/events/src/nsDOMXULCommandEvent.cpp(52) : warning C4244: '=' : conversion from 'PRTime' to 'PRUint32', possible loss of data

Because PR_Now() returns PRTime (==PRInt64) and nsEvent::time is PRUint32.

http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/widget/public/nsGUIEvent.h&rev=3.145&root=/cvsroot&mark=386-388#356
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/nsprpub/pr/include/prtime.h&rev=3.10&root=/cvsroot#164

(also, the comment in nsEvent looks incorrect -- it doesn't match the
comment for PR_Now() anyway)

Can we change the type for nsEvent::time to PRTime?

Comment 1

[Boris Zbarsky [:bzbarsky]]

There's a known mess with nsEvent::time and what it _should_ be.  Per DOM3 Events spec, this member should in fact be in milliseconds, last I checked.  See http://www.w3.org/TR/2003/NOTE-DOM-Level-3-Events-20031107/events.html#Events-Event-timeStamp

None of which is to say we might not want a 64-bit value here so we can have this working past 2038.  But the point is that assigning PR_Now() into it (as we do in all sorts of places, including nsDOMEvent::nsDOMevent) is bogus.

Note that I'm sure we have bugs covering this...

Comment 2

[Wayne Mery (:wsmwk)]

(In reply to comment #1)
> There's a known mess with nsEvent::time and what it _should_ be.  Per DOM3
> Events spec, this member should in fact be in milliseconds, last I checked. 
> See
> http://www.w3.org/TR/2003/NOTE-DOM-Level-3-Events-20031107/events.html#Events-Event-timeStamp
> 
> None of which is to say we might not want a 64-bit value here so we can have
> this working past 2038.  But the point is that assigning PR_Now() into it (as
> we do in all sorts of places, including nsDOMEvent::nsDOMevent) is bogus.
> 
> Note that I'm sure we have bugs covering this...

as in, the non-perf aspects of bug 58310?

Comment 3

[Atul Aggarwal]

There is no such warning in nsDOMXULCommandEvent.cpp (in current code base). Also verified that no warning is there in nighly logs of mac, windows and linux (debug also).