Closed Bug 591845 Opened 11 years ago Closed 11 years ago

New Zealand timezones incorrect


(Core :: JavaScript Engine, defect)

Windows 2000
Not set



Tracking Status
blocking2.0 --- beta7+


(Reporter: darktrojan, Assigned: bhackett1024)



(Keywords: regression, Whiteboard: fixed-in-tracemonkey)


(1 file)

* Set Windows timezone to New Zealand (seems to be a Windows-only thing)
* javascript:alert(new Date())

Expected: Sun Aug 29 2010 14:12:21 GMT+1200 (New Zealand Standard Time)
Got:      Sun Aug 29 2010 14:12:21 GMT-1200 (New Zealand Daylight Time)

Discussed this on IRC, and we suspect bug 578259.
blocking2.0: --- → ?
> Expected: Sun Aug 29 2010 14:12:21 GMT+1200 (New Zealand Standard Time)

I'm really on top of things today. I meant: 
Mon Aug 30 2010 14:12:21 GMT+1200 (New Zealand Standard Time)
Blair confirmed that reports about the same value on his Windows and OS X boxes (+/- a few seconds), but that that the |new Date()| string date was 12 hours off as reported above.
Blocks: 578259
Keywords: regression
Is there a Windows box available for testing fixes?  I only have an OSX box, and it seems to be doing the right thing:

Mon Aug 30 2010 15:54:52 GMT+1200 (NZST)
Windows and OS X behave differently on this because they disagree over whether New Zealand was observing daylight savings time on January 1, 1970.  The basic problem, though, is that bug 578259 is not accounting correctly for locations that were observing daylight savings time at the epoch.  It's also doing the wrong thing for the Kingdom of Tonga, which is at UTC+1300.  Working on a fix for both of these.
Attached patch fixSplinter Review
Assignee: general → bhackett1024
Attachment #470598 - Flags: review?(jwalden+bmo)
Attachment #470598 - Flags: review?(jwalden+bmo) → review?(brendan)
Comment on attachment 470598 [details] [diff] [review]

>+/* Get the local time. localtime_r is preferred as it is reentrant. */
>+static inline bool
>+computeLocalTime(time_t local, struct tm *ptm)

Nit: prevailing style for static helpers (deviations in jstracer.cpp aside) uses StudlyCaps not camelCaps.

>+    return localtime_r(&local, ptm);
>+    struct tm *otm = localtime(&local);
>+    if (!otm)

Is localtime fallible? My Mac OS X man page says it always returns the address of its static buffer.

>     /*
>      * Get the difference between this time zone and GMT, by checking the local
>-     * time at the epoch.
>+     * time for days 0 and 180 of 1970, using a date for which daylight
>+     * savings time was not in effect.

Nit: rewrap so savings goes on the same line as daylight, it fits.

>      */
>-    time_t local = 0;
>+    JSInt32 day = 0;

Nit: do not use the JSInt32, etc. types, they are a historical relic from 1998. Use int32 if width matters (it doesn't here, right?) or just int (for best performance in general for locals, assuming sane compiler and target arch).

>+        if (!computeLocalTime(PRMJ_DAY_SECONDS * day, &tm))
>+            return 0;

Failure here means epoch start time, but I still don't know how localtime can fail (return null).

>-    struct tm *ptm = localtime(&local);
>-    if (!ptm)
>+    if (!computeLocalTime(static_cast<time_t>(localTimeSeconds), &tm))
>         return 0;

Ditto re: fallibility.

r=me with fixes.

Attachment #470598 - Flags: review?(brendan) → review+
I've been using:

Which says localtime and localtime_r can both return NULL.  I don't know whether that ever actually occurs.

localtime return value checks are still there, talked about this with Brendan on IRC.

bhackett: brendan: also, did you look at that localtime thing?
[2:43pm] bhackett: says it can EOVERFLOW
[2:43pm] brendan: bhackett: oops, behind on bugmail
[2:43pm] brendan: bhackett: ok, i gave r+ modulo fixes
[2:43pm] brendan: sounds like no fix needed there (hard to believe)
[2:44pm] bhackett: I don't think it's really possible
[2:44pm] bhackett: still don't want to SEGV if the OS is being weird
blocking2.0: ? → beta6+
Whiteboard: fixed-in-tracemonkey
Closed: 11 years ago
Resolution: --- → FIXED
Duplicate of this bug: 599802
Did this bug fix the test failures from bug 530974? If not, how was it tested?
OS: Windows Vista → Windows 2000
I don't know if this fixes 530974.  This is fixing a regression from bug 578259, which postdates 530974; I tested by building on a Windows machine (which is where this bug manifests), making sure that 'new Date()' produces the correct time for New Zealand and nearby timezones, and that the date-format-* gave the same set of times.  Timezone-related bugs are hard to write tests for, as JS always gets the timezone from the OS.
Duplicate of this bug: 600320
Target Milestone: mozilla2.0b5 → mozilla2.0b7
You need to log in before you can comment on or make changes to this bug.