Project: v8 Issues People Development process History Sign in
New issue
Advanced search Search tips
Issue 2137 Timezone computations is wrong for Lord Howe Island
Starred by 7 users Reported by floitsch@google.com, May 16 2012 Back to list
Status: Assigned
Owner:
Cc:
Components:
HW: ----
OS: ----
Priority: 3
Type: Bug



Sign in to add a comment
The daylights saving time for Lord Howe Island is "Lord Howe Daylight Time" which is only off by 30 minutes. V8 assumes that all daylight savings offsets are one hour.
Note that Lord Howe Island is on the southern hemisphere and that summer time is hence in the winter.

$ rlwrap v8
V8 version 3.4.4 (candidate) [console: dumb]
d8> new Date()
Thu May 17 2012 03:33:15 GMT+1030 (LHST)
d8> new Date(2012, 0, 12)
Thu Jan 12 2012 00:00:00 GMT+1130 (LHST)

The latter date (2012, 0, 12) should be in GMT+1100. The "LHST" should probably also change to "LHDT", but Firefox doesn't do that either.
 
The OP's comment "Lord Howe Island is on the southern hemisphere and that summer time is hence in the winter" may be confusing.   Summer time in LHI is in the southern summer (what a surprise!).  Which happens to be at _roughly_ the same time (official start and end dates will probably differ) as northern winter.  
Comment 3 by floitsch@google.com, May 21 2012
Small addition: LHDT does not seem to be an agreed upon abbreviation.
Wikipedia does not even list it (http://en.wikipedia.org/wiki/List_of_time_zone_abbreviations), and Linux only uses LHST.

And thanks for the clarification on summer/winter :)
Comment 4 by floitsch@google.com, May 21 2012
And since I have already looked it up: the bad code is in platform-linux.cc:
OS::LocalTimeOffset which assumes that daylight savings is already 1 hour.

Maybe it would be enough to just return the static global timezone instead? If not, I would be interested to hear, why not.
Comment 5 by mj1...@gmail.com, Aug 1 2013
If it is related to half-hour offsets, wouldn't other places be failing as well?  For example, India is UTC+5:30.  There are many others with :30 offsets, and a few with :45 offsets also.
Owner: u...@chromium.org
Labels: Area-Runtime Type-Bug Priority-Low
Status: Assigned
Cc: js...@chromium.org littledan@chromium.org
It looks to me like the offending code is actually in a number of places: both  src/base/platform/platform-posix.cc and src/base/platform/platform-win32.cc have logic in their implementation of OS::DaylightSavingsOffset which assumes that the offset is either no time or one hour. (That code in platform-linux.cc is actually (incorrectly) correcting a different piece of time data, I believe.) An issue that's leading to this is that both Linux and Windows are making syscalls which return a boolean is_dst value.

I am not sure whether all platforms have an OS API to get the proper DST offset (hopefully they do), but ICU does seem to have a simple API for this: http://www.icu-project.org/apiref/icu4c/classicu_1_1TimeZone.html#a026e66c6f698915089c3a3ab29dfd80b
Labels: Priority-3
Project Member Comment 10 by bugdroid1@chromium.org, Apr 11
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/b213f2399038a615cdfbfa0201cddc113d304018

commit b213f2399038a615cdfbfa0201cddc113d304018
Author: littledan <littledan@chromium.org>
Date: Tue Apr 11 11:37:31 2017

[date] Add ICU backend for timezone info behind a flag

This patch implements a timezone backend which is based on ICU, rather
than operating system calls. It can be turned on by passing the
--icu-timezone-data flag. The goal here is to take advantage of ICU's
data, which is more complete than the data that some system calls expose.
For example, without any special code, this patch fixes the time zone
of Lord Howe Island to have a correct 30 minute DST offset, rather than
60 minutes as the OS backends assume it to have.

Unfortunately, the parenthized timezone name in Date.prototype.toString()
differs across platforms. This patch chooses the long timezone name,
which matches Windows behavior and might be the most intelligible, but
the web compatibility impact is unclear.

BUG=v8:6031,v8:2137,v8:6076

Review-Url: https://codereview.chromium.org/2724373002
Cr-Commit-Position: refs/heads/master@{#44562}

[modify] https://crrev.com/b213f2399038a615cdfbfa0201cddc113d304018/src/date.cc
[modify] https://crrev.com/b213f2399038a615cdfbfa0201cddc113d304018/src/date.h
[modify] https://crrev.com/b213f2399038a615cdfbfa0201cddc113d304018/src/flag-definitions.h
[modify] https://crrev.com/b213f2399038a615cdfbfa0201cddc113d304018/src/i18n.cc
[modify] https://crrev.com/b213f2399038a615cdfbfa0201cddc113d304018/src/i18n.h
[add] https://crrev.com/b213f2399038a615cdfbfa0201cddc113d304018/test/mjsunit/icu-date-lord-howe.js
[add] https://crrev.com/b213f2399038a615cdfbfa0201cddc113d304018/test/mjsunit/icu-date-to-string.js
[modify] https://crrev.com/b213f2399038a615cdfbfa0201cddc113d304018/test/mjsunit/mjsunit.status
[modify] https://crrev.com/b213f2399038a615cdfbfa0201cddc113d304018/test/mjsunit/testcfg.py
[modify] https://crrev.com/b213f2399038a615cdfbfa0201cddc113d304018/tools/testrunner/local/commands.py
[modify] https://crrev.com/b213f2399038a615cdfbfa0201cddc113d304018/tools/testrunner/local/execution.py
[modify] https://crrev.com/b213f2399038a615cdfbfa0201cddc113d304018/tools/testrunner/objects/testcase.py

Project Member Comment 11 by bugdroid1@chromium.org, Apr 11
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/13ad50811024ace5623d5d4d13cea4ef21f4affd

commit 13ad50811024ace5623d5d4d13cea4ef21f4affd
Author: machenbach <machenbach@chromium.org>
Date: Tue Apr 11 12:07:29 2017

Revert of [date] Add ICU backend for timezone info behind a flag (patchset #17 id:320001 of https://codereview.chromium.org/2724373002/ )

Reason for revert:
Breaks noi18n:
https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20noi18n%20-%20debug/builds/13314

Original issue's description:
> [date] Add ICU backend for timezone info behind a flag
>
> This patch implements a timezone backend which is based on ICU, rather
> than operating system calls. It can be turned on by passing the
> --icu-timezone-data flag. The goal here is to take advantage of ICU's
> data, which is more complete than the data that some system calls expose.
> For example, without any special code, this patch fixes the time zone
> of Lord Howe Island to have a correct 30 minute DST offset, rather than
> 60 minutes as the OS backends assume it to have.
>
> Unfortunately, the parenthized timezone name in Date.prototype.toString()
> differs across platforms. This patch chooses the long timezone name,
> which matches Windows behavior and might be the most intelligible, but
> the web compatibility impact is unclear.
>
> BUG=v8:6031,v8:2137,v8:6076
>
> Review-Url: https://codereview.chromium.org/2724373002
> Cr-Commit-Position: refs/heads/master@{#44562}
> Committed: https://chromium.googlesource.com/v8/v8/+/b213f2399038a615cdfbfa0201cddc113d304018

TBR=ulan@chromium.org,jshin@chromium.org,jgruber@chromium.org,littledan@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:6031,v8:2137,v8:6076

Review-Url: https://codereview.chromium.org/2811103002
Cr-Commit-Position: refs/heads/master@{#44565}

[modify] https://crrev.com/13ad50811024ace5623d5d4d13cea4ef21f4affd/src/date.cc
[modify] https://crrev.com/13ad50811024ace5623d5d4d13cea4ef21f4affd/src/date.h
[modify] https://crrev.com/13ad50811024ace5623d5d4d13cea4ef21f4affd/src/flag-definitions.h
[modify] https://crrev.com/13ad50811024ace5623d5d4d13cea4ef21f4affd/src/i18n.cc
[modify] https://crrev.com/13ad50811024ace5623d5d4d13cea4ef21f4affd/src/i18n.h
[delete] https://crrev.com/fc7c2c5535dae42449591cc1d59c6316928f2dc2/test/mjsunit/icu-date-lord-howe.js
[delete] https://crrev.com/fc7c2c5535dae42449591cc1d59c6316928f2dc2/test/mjsunit/icu-date-to-string.js
[modify] https://crrev.com/13ad50811024ace5623d5d4d13cea4ef21f4affd/test/mjsunit/mjsunit.status
[modify] https://crrev.com/13ad50811024ace5623d5d4d13cea4ef21f4affd/test/mjsunit/testcfg.py
[modify] https://crrev.com/13ad50811024ace5623d5d4d13cea4ef21f4affd/tools/testrunner/local/commands.py
[modify] https://crrev.com/13ad50811024ace5623d5d4d13cea4ef21f4affd/tools/testrunner/local/execution.py
[modify] https://crrev.com/13ad50811024ace5623d5d4d13cea4ef21f4affd/tools/testrunner/objects/testcase.py

Project Member Comment 12 by bugdroid1@chromium.org, Apr 11
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/c1a9e556cad0365dcf0860f6aded31dd49deb43b

commit c1a9e556cad0365dcf0860f6aded31dd49deb43b
Author: littledan <littledan@chromium.org>
Date: Tue Apr 11 13:17:29 2017

Reland of [date] Add ICU backend for timezone info behind a flag (patchset #1 id:1 of https://codereview.chromium.org/2811103002/ )

Reason for revert:
Reland with tests marked as off in no-i18n mode

Original issue's description:
> Revert of [date] Add ICU backend for timezone info behind a flag (patchset #17 id:320001 of https://codereview.chromium.org/2724373002/ )
>
> Reason for revert:
> Breaks noi18n:
> https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20noi18n%20-%20debug/builds/13314
>
> Original issue's description:
> > [date] Add ICU backend for timezone info behind a flag
> >
> > This patch implements a timezone backend which is based on ICU, rather
> > than operating system calls. It can be turned on by passing the
> > --icu-timezone-data flag. The goal here is to take advantage of ICU's
> > data, which is more complete than the data that some system calls expose.
> > For example, without any special code, this patch fixes the time zone
> > of Lord Howe Island to have a correct 30 minute DST offset, rather than
> > 60 minutes as the OS backends assume it to have.
> >
> > Unfortunately, the parenthized timezone name in Date.prototype.toString()
> > differs across platforms. This patch chooses the long timezone name,
> > which matches Windows behavior and might be the most intelligible, but
> > the web compatibility impact is unclear.
> >
> > BUG=v8:6031,v8:2137,v8:6076
> >
> > Review-Url: https://codereview.chromium.org/2724373002
> > Cr-Commit-Position: refs/heads/master@{#44562}
> > Committed: https://chromium.googlesource.com/v8/v8/+/b213f2399038a615cdfbfa0201cddc113d304018
>
> TBR=ulan@chromium.org,jshin@chromium.org,jgruber@chromium.org,littledan@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=v8:6031,v8:2137,v8:6076
>
> Review-Url: https://codereview.chromium.org/2811103002
> Cr-Commit-Position: refs/heads/master@{#44565}
> Committed: https://chromium.googlesource.com/v8/v8/+/13ad50811024ace5623d5d4d13cea4ef21f4affd

TBR=ulan@chromium.org,jshin@chromium.org,jgruber@chromium.org,machenbach@chromium.org
BUG=v8:6031,v8:2137,v8:6076

Review-Url: https://codereview.chromium.org/2813863002
Cr-Commit-Position: refs/heads/master@{#44575}

[modify] https://crrev.com/c1a9e556cad0365dcf0860f6aded31dd49deb43b/src/date.cc
[modify] https://crrev.com/c1a9e556cad0365dcf0860f6aded31dd49deb43b/src/date.h
[modify] https://crrev.com/c1a9e556cad0365dcf0860f6aded31dd49deb43b/src/flag-definitions.h
[modify] https://crrev.com/c1a9e556cad0365dcf0860f6aded31dd49deb43b/src/i18n.cc
[modify] https://crrev.com/c1a9e556cad0365dcf0860f6aded31dd49deb43b/src/i18n.h
[add] https://crrev.com/c1a9e556cad0365dcf0860f6aded31dd49deb43b/test/mjsunit/icu-date-lord-howe.js
[add] https://crrev.com/c1a9e556cad0365dcf0860f6aded31dd49deb43b/test/mjsunit/icu-date-to-string.js
[modify] https://crrev.com/c1a9e556cad0365dcf0860f6aded31dd49deb43b/test/mjsunit/mjsunit.status
[modify] https://crrev.com/c1a9e556cad0365dcf0860f6aded31dd49deb43b/test/mjsunit/testcfg.py
[modify] https://crrev.com/c1a9e556cad0365dcf0860f6aded31dd49deb43b/tools/testrunner/local/commands.py
[modify] https://crrev.com/c1a9e556cad0365dcf0860f6aded31dd49deb43b/tools/testrunner/local/execution.py
[modify] https://crrev.com/c1a9e556cad0365dcf0860f6aded31dd49deb43b/tools/testrunner/objects/testcase.py

Sign in to add a comment