New Zealand timezones incorrect

RESOLVED FIXED in mozilla2.0b7

Status

()

--
major
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: darktrojan, Assigned: bhackett)

Tracking

({regression})

Trunk
mozilla2.0b7
x86
Windows 2000
regression
Points:
---

Firefox Tracking Flags

(blocking2.0 beta7+)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 attachment)

(Reporter)

Description

8 years ago
STR:
* 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.
(Reporter)

Updated

8 years ago
blocking2.0: --- → ?
Severity: blocker → major
(Reporter)

Comment 1

8 years ago
> 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 Date.now() 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
(Assignee)

Comment 3

8 years ago
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)
(Assignee)

Comment 4

8 years ago
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.
(Assignee)

Comment 5

8 years ago
Created attachment 470598 [details] [diff] [review]
fix
Assignee: general → bhackett1024
Attachment #470598 - Flags: review?(jwalden+bmo)
(Assignee)

Updated

8 years ago
Attachment #470598 - Flags: review?(jwalden+bmo) → review?(brendan)
Comment on attachment 470598 [details] [diff] [review]
fix

>+/* 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.

>+{
>+#ifdef HAVE_LOCALTIME_R
>+    return localtime_r(&local, ptm);
>+#else
>+    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.
..12345678901234567890123456789012345678901234567890123456789012345678901234567890

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).

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

Ditto re: fallibility.

r=me with fixes.

/be
Attachment #470598 - Flags: review?(brendan) → review+
(Assignee)

Comment 7

8 years ago
I've been using:

http://www.opengroup.org/onlinepubs/000095399/functions/localtime.html

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

Comment 8

8 years ago
http://hg.mozilla.org/tracemonkey/rev/bb8eeb9890a7

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: opengroup.org 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

Updated

8 years ago
blocking2.0: ? → beta6+

Updated

8 years ago
Whiteboard: fixed-in-tracemonkey

Comment 9

8 years ago
http://hg.mozilla.org/mozilla-central/rev/bb8eeb9890a7
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Assignee)

Updated

8 years ago
Duplicate of this bug: 599802

Comment 11

8 years ago
Did this bug fix the test failures from bug 530974? If not, how was it tested?
OS: Windows Vista → Windows 2000
(Assignee)

Comment 12

8 years ago
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.