New issue
Advanced search Search tips
Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
HW: ----
NextAction: ----
OS: ----
Priority: 2
Type: Bug

Blocked on:
issue 6031
issue chromium:774376



Sign in to add a comment

Regularize Date.prototype.toString across platforms

Project Member Reported by littledan@chromium.org, Mar 10 2017

Issue description

Date.prototype.toString() gets its timezone name from the OS. According to Jungshik (in https://codereview.chromium.org/2726253002/):

> I'm really tempted to move away from the current output format for Date.toString() which is different on different platforms.  On Windows, we show a rather long descriptive and *localized* name enclosed by parens. On Linux, we show 'cryptic' 3-4 letters long abbreviations.

Unfortunately, the spec isn't much guidance here; probably this comes from the historic variability (https://tc39.github.io/ecma262/#sec-todatestring):

> Return an implementation-dependent String value that represents tv as a date and time in the current time zone using a convenient, human-readable form.

What do other browsers do here? I don't have a setup to run other browsers, but from a quick skim of the source code, it looks like Firefox calls strftime on all platforms, whereas JSC and ChakraCore make similar calls to what V8 currently makes. (According to Windows docs, %Z in strftime returns "Either the time-zone name or time zone abbreviation, depending on registry settings; no characters if time zone is unknown").

A couple thoughts of actions from here:
- I don't have a good understanding of the compat impact of any sort of change here. Although the desktop web would predominantly get the long, localized timezone names, the mobile web would get almost entirely the short names from the tz database. I think people are likely to really use this string, e.g., see the second answer to the first Google hit for "how to get the current timezone in javascript": http://stackoverflow.com/a/15304657 (the best answer, #4 there, is not available in the second-to-last version of Safari). Is it OK to just choose one of the two (e.g., as Jungshik suggests, do current Windows-like behavior on all platforms), or should we preserve the current platform-dependent behavior?
- Seems like Date.prototype.toString is a fingerprinting vector along a couple axes--which OS is running, and if it's Windows, the locale. The HTML spec documents fingerprinting vectors, so it seems to me like this deserves spec documentation as well.
- Aside from the rendering the timezone name in parens, Date.prototype.toString seems to be the same between browsers. It might be a good idea to lock this down in the spec so things don't get any worse.
 
One extra twist for Firefox:
If the localized time zone string contains any non-ASCII characters, it's not displayed [1]. For example with a German locale and a European time zone like Europe/Berlin, Firefox returns "Fri Mar 10 2017 14:32:01 GMT+0100" instead of "Fri Mar 10 2017 14:32:01 GMT+0100 (Mitteleuropäische Zeit)", because of the "ä" in "Mitteleuropäische".

[1] http://searchfox.org/mozilla-central/rev/7cb75d87753de9103253e34bc85592e26378f506/js/src/jsdate.cpp#2637-2651

Comment 2 by js...@chromium.org, Mar 10 2017

re comment #1: I guess that's not intended. Perhaps, it's a bug in Firefox (they assumed that timezone names are always ASCII).  

Anyway, thanks, Dan, for starting this discussion. 

If I were not to worry about web-compat, I'd go with ISO 8601.  
As much as I like that idea too, I don't think we can go with ISO 8601--it's significantly different from the cross-browser consensus of how hot print dates, and there are lots of Stack Overflow posts which talk about parsing this output. AFAICT the only thing that differs between browsers is the timezone name.

If we want to go to something analogous to IS0 8601 for the timezone name, I guess that would be omitting the parenthesized portion, as Firefox apparently occasionally does, as ISO 8601 doesn't include any sort of readable name, just the offset from UTC. I don't know whether this is more web-compatible, but I think it's likelier to be web-compatible than switching to ISO 8601.

I filed an upstream bug at https://github.com/tc39/ecma262/issues/845 to discuss this issue.
Labels: Priority-2
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 11 2017

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 6 by bugdroid1@chromium.org, Apr 11 2017

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 7 by bugdroid1@chromium.org, Apr 11 2017

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

Comment 8 by js...@chromium.org, Jul 20 2017

Blockedon: 6031

Comment 9 by js...@chromium.org, Jul 20 2017

Blocking: -6031
Project Member

Comment 10 by bugdroid1@chromium.org, Sep 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/d9a25842d3aba82ead5e06c7fd7cb4896e6ad202

commit d9a25842d3aba82ead5e06c7fd7cb4896e6ad202
Author: Jungshik Shin <jshin@chromium.org>
Date: Mon Sep 11 16:56:16 2017

Enable icu-timezone-data by default

This will introduce a new behavior on POSIX(-like) platforms. Timezone
names inside parentheses after GMT offset will not be 3-4 letter
abbreviation any longer. They'll be human-readable names in the current
default locale. This matches the current Windows behavior.

new Date(2017, 5, 22).toString()
new Date(2017, 11, 22).toString()

Current:

Thu Jun 22 2017 00:00:00 GMT-0700 (PDT)
Fri Dec 22 2017 00:00:00 GMT-0800 (PST)

New in en-US locale:

Thu Jun 22 2017 00:00:00 GMT-0700 (Pacific Daylight Time)
Fri Dec 22 2017 00:00:00 GMT-0800 (Pacific Standard Time)

New in German locale:

Thu Jun 22 2017 00:00:00 GMT-0700 (Nordamerikanische Westküsten-Sommerzeit)
Fri Dec 22 2017 00:00:00 GMT-0800 (Nordamerikanische Westküsten-Normalzeit)

BUG= v8:6031 ,  v8:2137 , v8:6076
TEST=mjsunit/icu-date-lord-howe.js, mjsunit/icu-date-to-string.js

Change-Id: I4e7fd8b3ddae5c7779e220c4c101e45904fcdc01
Reviewed-on: https://chromium-review.googlesource.com/625164
Commit-Queue: Jungshik Shin <jshin@chromium.org>
Reviewed-by: Daniel Ehrenberg <littledan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47953}
[modify] https://crrev.com/d9a25842d3aba82ead5e06c7fd7cb4896e6ad202/src/flag-definitions.h

Project Member

Comment 11 by bugdroid1@chromium.org, Oct 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/28ef8dc7009d57de2782668ed00dff34e31dee2d

commit 28ef8dc7009d57de2782668ed00dff34e31dee2d
Author: Jungshik Shin <jshin@chromium.org>
Date: Wed Oct 11 23:21:37 2017

Revert "Enable icu-timezone-data by default"

This reverts commit d9a25842d3aba82ead5e06c7fd7cb4896e6ad202.

Reason for revert: 

I'm reverting this CL for a few reasons. #2 is the most significant and I should have thought of that before making a switch. Sorry for that.   

1) perf-regression:  http://crbug.com/769706 
2)  http://crbug.com/612010  : ICU timezone update is not propagated to zygote process so that new tabs will hold on to an old timezone even after a timezone change on Linux and Chrome OS. 
3)  http://crbug.com/754053  : OS timezone detection issues on macOS 10.13, Ubutu 16, RHEL 7, SuSe Linux 12 or newer. ; it's being fixed. So, it actually ok.  
4)  http://crbug.com/771868  : timezone wrong in gmail: If it's due to #3, we're fine because it's fixed. If not, we need to look more.

Original change's description:
> Enable icu-timezone-data by default
> 
> This will introduce a new behavior on POSIX(-like) platforms. Timezone
> names inside parentheses after GMT offset will not be 3-4 letter
> abbreviation any longer. They'll be human-readable names in the current
> default locale. This matches the current Windows behavior.
> 
> new Date(2017, 5, 22).toString()
> new Date(2017, 11, 22).toString()
> 
> Current:
> 
> Thu Jun 22 2017 00:00:00 GMT-0700 (PDT)
> Fri Dec 22 2017 00:00:00 GMT-0800 (PST)
> 
> New in en-US locale:
> 
> Thu Jun 22 2017 00:00:00 GMT-0700 (Pacific Daylight Time)
> Fri Dec 22 2017 00:00:00 GMT-0800 (Pacific Standard Time)
> 
> New in German locale:
> 
> Thu Jun 22 2017 00:00:00 GMT-0700 (Nordamerikanische Westküsten-Sommerzeit)
> Fri Dec 22 2017 00:00:00 GMT-0800 (Nordamerikanische Westküsten-Normalzeit)
> 
> BUG= v8:6031 ,  v8:2137 , v8:6076
> TEST=mjsunit/icu-date-lord-howe.js, mjsunit/icu-date-to-string.js
> 
> Change-Id: I4e7fd8b3ddae5c7779e220c4c101e45904fcdc01
> Reviewed-on: https://chromium-review.googlesource.com/625164
> Commit-Queue: Jungshik Shin <jshin@chromium.org>
> Reviewed-by: Daniel Ehrenberg <littledan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#47953}

TBR=adamk@chromium.org,littledan@chromium.org,jshin@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  v8:6031 ,  v8:2137 , v8:6076,  chromium:769706 ,  chromium:612010 ,  chromium:771868 
Change-Id: I60d75467ee21975d3a235344b01c0d2d44a7da96
Reviewed-on: https://chromium-review.googlesource.com/713404
Reviewed-by: Adam Klein <adamk@chromium.org>
Commit-Queue: Jungshik Shin <jshin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48478}
[modify] https://crrev.com/28ef8dc7009d57de2782668ed00dff34e31dee2d/src/flag-definitions.h

Comment 12 by js...@chromium.org, Oct 13 2017

Blockedon: chromium:774376
Project Member

Comment 13 by bugdroid1@chromium.org, Oct 16 2017

Labels: merge-merged-6.3
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/d7343703bc6a3c1dbd5ead201186c520ffc9e47b

commit d7343703bc6a3c1dbd5ead201186c520ffc9e47b
Author: Jungshik Shin <jshin@chromium.org>
Date: Mon Oct 16 20:43:16 2017

Merged: Revert "Enable icu-timezone-data by default"

This reverts commit d9a25842d3aba82ead5e06c7fd7cb4896e6ad202.
This is for 6.3 branch (to go with Chromium 63 branch). 

Reason for revert:

I'm reverting this CL for a few reasons. #2 is the most significant and
I should have thought of that before making a switch. Sorry for that.

1) perf-regression:  http://crbug.com/769706 
2)  http://crbug.com/612010  : ICU timezone update is not propagated to
zygote process so that new tabs will hold on to an old timezone even
after a timezone change on Linux and Chrome OS.

3)   http://crbug.com/754053   : OS timezone detection issues on macOS
10.13, Ubutu 16, RHEL 7, SuSe Linux 12 or newer. ; it's being fixed. So,
it actually ok.

4)   http://crbug.com/771868   : timezone wrong in gmail: If it's due to

Revision: 28ef8dc7009d

BUG= chromium:612010 , chromium:769706 , chromium:771868 , v8:2137 , v8:6031 ,v8:6076
LOG=N
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
R=adamk@chromium.org, hablich@chromium.org

Change-Id: Id0093c2bc69e5bc1d3b7147b7bbcc633ed625a45
Reviewed-on: https://chromium-review.googlesource.com/721919
Reviewed-by: Adam Klein <adamk@chromium.org>
Commit-Queue: Jungshik Shin <jshin@chromium.org>
Cr-Commit-Position: refs/branch-heads/6.3@{#7}
Cr-Branched-From: 094a7c93dcdcd921de3883ba4674b7e1a0feffbe-refs/heads/6.3.292@{#1}
Cr-Branched-From: 18b8fbb528a8021e04a029e06eafee50b918bce0-refs/heads/master@{#48432}
[modify] https://crrev.com/d7343703bc6a3c1dbd5ead201186c520ffc9e47b/src/flag-definitions.h

Project Member

Comment 14 by bugdroid1@chromium.org, Oct 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/3b1cfa38bcd616609f263ac3b6721c0214c45d19

commit 3b1cfa38bcd616609f263ac3b6721c0214c45d19
Author: Michael Hablich <hablich@chromium.org>
Date: Tue Oct 17 16:11:43 2017

Revert "Merged: Revert "Enable icu-timezone-data by default""

This reverts commit d7343703bc6a3c1dbd5ead201186c520ffc9e47b.

Reason for revert: broke some branch builders like https://build.chromium.org/p/client.v8.branches/builders/V8%20arm%20-%20sim%20-%20beta%20branch%20-%20debug

Original change's description:
> Merged: Revert "Enable icu-timezone-data by default"
> 
> This reverts commit d9a25842d3aba82ead5e06c7fd7cb4896e6ad202.
> This is for 6.3 branch (to go with Chromium 63 branch). 
> 
> Reason for revert:
> 
> I'm reverting this CL for a few reasons. #2 is the most significant and
> I should have thought of that before making a switch. Sorry for that.
> 
> 1) perf-regression:  http://crbug.com/769706 
> 2)  http://crbug.com/612010  : ICU timezone update is not propagated to
> zygote process so that new tabs will hold on to an old timezone even
> after a timezone change on Linux and Chrome OS.
> 
> 3)   http://crbug.com/754053   : OS timezone detection issues on macOS
> 10.13, Ubutu 16, RHEL 7, SuSe Linux 12 or newer. ; it's being fixed. So,
> it actually ok.
> 
> 4)   http://crbug.com/771868   : timezone wrong in gmail: If it's due to
> 
> Revision: 28ef8dc7009d
> 
> BUG= chromium:612010 , chromium:769706 , chromium:771868 , v8:2137 , v8:6031 ,v8:6076
> LOG=N
> NOTRY=true
> NOPRESUBMIT=true
> NOTREECHECKS=true
> R=​adamk@chromium.org, hablich@chromium.org
> 
> Change-Id: Id0093c2bc69e5bc1d3b7147b7bbcc633ed625a45
> Reviewed-on: https://chromium-review.googlesource.com/721919
> Reviewed-by: Adam Klein <adamk@chromium.org>
> Commit-Queue: Jungshik Shin <jshin@chromium.org>
> Cr-Commit-Position: refs/branch-heads/6.3@{#7}
> Cr-Branched-From: 094a7c93dcdcd921de3883ba4674b7e1a0feffbe-refs/heads/6.3.292@{#1}
> Cr-Branched-From: 18b8fbb528a8021e04a029e06eafee50b918bce0-refs/heads/master@{#48432}

TBR=adamk@chromium.org,hablich@chromium.org,jshin@chromium.org

Change-Id: I8b30f8f9c63b93cb000facbf910ed111ca3f9ab2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  chromium:612010 ,  chromium:769706 ,  chromium:771868 ,  v8:2137 ,  v8:6031 , v8:6076
Reviewed-on: https://chromium-review.googlesource.com/723324
Reviewed-by: Michael Hablich <hablich@chromium.org>
Commit-Queue: Michael Hablich <hablich@chromium.org>
Cr-Commit-Position: refs/branch-heads/6.3@{#18}
Cr-Branched-From: 094a7c93dcdcd921de3883ba4674b7e1a0feffbe-refs/heads/6.3.292@{#1}
Cr-Branched-From: 18b8fbb528a8021e04a029e06eafee50b918bce0-refs/heads/master@{#48432}
[modify] https://crrev.com/3b1cfa38bcd616609f263ac3b6721c0214c45d19/src/flag-definitions.h

Project Member

Comment 15 by bugdroid1@chromium.org, Oct 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/9685d9c25a45f7902fe6250b64ca61abeea1e9f5

commit 9685d9c25a45f7902fe6250b64ca61abeea1e9f5
Author: Jungshik Shin <jshin@chromium.org>
Date: Tue Oct 17 20:55:47 2017

Revert "Revert "Merged: Revert "Enable icu-timezone-data by default"""

This reverts commit 3b1cfa38bcd616609f263ac3b6721c0214c45d19.

Reason for revert: ICU was not rolled in 6.3 branch leading invalid-locale test failure (that was added to test an ICU fix). Now, ICU is rolled in 6.3 branch ( https://chromium-review.googlesource.com/c/v8/v8/+/723564 ). 

Original change's description:
> Revert "Merged: Revert "Enable icu-timezone-data by default""
> 
> This reverts commit d7343703bc6a3c1dbd5ead201186c520ffc9e47b.
> 
> Reason for revert: broke some branch builders like https://build.chromium.org/p/client.v8.branches/builders/V8%20arm%20-%20sim%20-%20beta%20branch%20-%20debug
> 
> Original change's description:
> > Merged: Revert "Enable icu-timezone-data by default"
> > 
> > This reverts commit d9a25842d3aba82ead5e06c7fd7cb4896e6ad202.
> > This is for 6.3 branch (to go with Chromium 63 branch). 
> > 
> > Reason for revert:
> > 
> > I'm reverting this CL for a few reasons. #2 is the most significant and
> > I should have thought of that before making a switch. Sorry for that.
> > 
> > 1) perf-regression:  http://crbug.com/769706 
> > 2)  http://crbug.com/612010  : ICU timezone update is not propagated to
> > zygote process so that new tabs will hold on to an old timezone even
> > after a timezone change on Linux and Chrome OS.
> > 
> > 3)   http://crbug.com/754053   : OS timezone detection issues on macOS
> > 10.13, Ubutu 16, RHEL 7, SuSe Linux 12 or newer. ; it's being fixed. So,
> > it actually ok.
> > 
> > 4)   http://crbug.com/771868   : timezone wrong in gmail: If it's due to
> > 
> > Revision: 28ef8dc7009d
> > 
> > BUG= chromium:612010 , chromium:769706 , chromium:771868 , v8:2137 , v8:6031 ,v8:6076
> > LOG=N
> > NOTRY=true
> > NOPRESUBMIT=true
> > NOTREECHECKS=true
> > R=​adamk@chromium.org, hablich@chromium.org
> > 
> > Change-Id: Id0093c2bc69e5bc1d3b7147b7bbcc633ed625a45
> > Reviewed-on: https://chromium-review.googlesource.com/721919
> > Reviewed-by: Adam Klein <adamk@chromium.org>
> > Commit-Queue: Jungshik Shin <jshin@chromium.org>
> > Cr-Commit-Position: refs/branch-heads/6.3@{#7}
> > Cr-Branched-From: 094a7c93dcdcd921de3883ba4674b7e1a0feffbe-refs/heads/6.3.292@{#1}
> > Cr-Branched-From: 18b8fbb528a8021e04a029e06eafee50b918bce0-refs/heads/master@{#48432}
> 
> TBR=adamk@chromium.org,hablich@chromium.org,jshin@chromium.org
> 
> Change-Id: I8b30f8f9c63b93cb000facbf910ed111ca3f9ab2
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  chromium:612010 ,  chromium:769706 ,  chromium:771868 ,  v8:2137 ,  v8:6031 , v8:6076
> Reviewed-on: https://chromium-review.googlesource.com/723324
> Reviewed-by: Michael Hablich <hablich@chromium.org>
> Commit-Queue: Michael Hablich <hablich@chromium.org>
> Cr-Commit-Position: refs/branch-heads/6.3@{#18}
> Cr-Branched-From: 094a7c93dcdcd921de3883ba4674b7e1a0feffbe-refs/heads/6.3.292@{#1}
> Cr-Branched-From: 18b8fbb528a8021e04a029e06eafee50b918bce0-refs/heads/master@{#48432}

TBR=adamk@chromium.org,hablich@chromium.org,jshin@chromium.org

Change-Id: I8f712f1e6eb1f14c703ffe9f1d63d23b4b4bb08e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  chromium:612010 ,  chromium:769706 ,  chromium:771868 ,  v8:2137 ,  v8:6031 , v8:6076
Reviewed-on: https://chromium-review.googlesource.com/723962
Reviewed-by: Michael Hablich <hablich@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Commit-Queue: Jungshik Shin <jshin@chromium.org>
Cr-Commit-Position: refs/branch-heads/6.3@{#21}
Cr-Branched-From: 094a7c93dcdcd921de3883ba4674b7e1a0feffbe-refs/heads/6.3.292@{#1}
Cr-Branched-From: 18b8fbb528a8021e04a029e06eafee50b918bce0-refs/heads/master@{#48432}
[modify] https://crrev.com/9685d9c25a45f7902fe6250b64ca61abeea1e9f5/src/flag-definitions.h

Project Member

Comment 16 by bugdroid1@chromium.org, Mar 7 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/ae314567068815d494ef206db54a887cdd614db7

commit ae314567068815d494ef206db54a887cdd614db7
Author: Jungshik Shin <jshin@chromium.org>
Date: Wed Mar 07 18:09:43 2018

Re-enable icu-timezone-data by default

icu-timezone-data was enabled before but reverted due to a perf issue.
(sunspider/date-format-totfe regressed;  crbug.com/769706  ).

However, my in-Chrome test of the same test [1] shows that there's virtually
no perf difference. See https://goo.gl/GX1jt6 .

This will introduce a new behavior on POSIX(-like) platforms. Timezone
names inside parentheses after GMT offset will not be 3-4 letter
abbreviation any longer. They'll be human-readable names in the current
default locale. This matches the current Windows behavior.

new Date(2017, 5, 22).toString()
new Date(2017, 11, 22).toString()

Current:

Thu Jun 22 2017 00:00:00 GMT-0700 (PDT)
Fri Dec 22 2017 00:00:00 GMT-0800 (PST)

New:

Thu Jun 22 2017 00:00:00 GMT-0700 (Pacific Daylight Time)
Fri Dec 22 2017 00:00:00 GMT-0800 (Pacific Standard Time)

This CL will be followed by
  https://chromium-review.googlesource.com/c/v8/v8/+/572148 to
implement https://github.com/tc39/ecma262/pull/778 .

[1] http://jungshik.github.io/v8/cr769706.html

BUG= v8:6031 ,  v8:2137 , v8:6076,  chromium:769706 
TEST=mjsunit/icu-date-lord-howe.js, mjsunit/icu-date-to-string.js

Change-Id: I22203670c3307a57fbf99e5f0a271dcbfbbef8fd
Reviewed-on: https://chromium-review.googlesource.com/857333
Commit-Queue: Jungshik Shin <jshin@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51791}
[modify] https://crrev.com/ae314567068815d494ef206db54a887cdd614db7/src/flag-definitions.h

Cc: -js...@chromium.org
Owner: js...@chromium.org
Status: Assigned (was: Available)
This can be declared as fixed.  The output of toString is the same across platforms. 

There's one related issue described in  bug 5714  comment 19 (https://bugs.chromium.org/p/v8/issues/detail?id=5714#c19 ). 

$ TZ=Europe/Istanbul out/rel/d8
V8 version 6.7.0 (candidate)
d8> new Date(2016,0,1)
Fri Jan 01 2016 00:00:00 GMT+0200 (GMT+03:00)

There are two issues here:
.

1) Istanbul does not have a descriptive display name in CLDR (even though it is a meta zone). As a result, fallback to GMT{+,-}hh:mm is used 

2) The offset in parens is 1-hr off because that name is cached from the current value in effect. Turkey changed the standard timezone offset (+2 to +3) and stopped using DST.  

Given that what goes inside parens is not likely to present a compat risk,  I'm tempted to use the Olson timezone ID.  Then, we don't have to worry about neither of issues. 

As for the 1st issue, there are other timezones missing 'descriptive display names'. 

As for the 2nd issue, there are other timezones for which timezone changes (splitting or mergeing) leads to a mismatch between  the cached descriptive name (obtained from the present time) and a timezone in effect for the time being displayed (future or past). 

Using Olson timezone ID inside parens can avoid both issues. 

Dan, what do you think of that?  

I filed a CLDR bug about missing metazone display names (https://unicode.org/cldr/trac/ticket/11043 ).  There are about 30 missing entries, but only two (Turkey and Urumqi metazones) are currently used. Other metazones were used in the past. 

Example for the 2nd issue in comment 17: 

<timezone type="Europe/Minsk">
        <usesMetazone to="1991-03-30 23:00" mzone="Moscow"/> 
        <usesMetazone to="2011-03-27 00:00" from="1991-03-30 23:00" mzone="Europe_Eastern"/>
        <usesMetazone to="2014-10-26 22:00" from="2011-03-27 00:00" mzone="Europe_Further_Eastern"/>
        <usesMetazone from="2014-10-26 22:00" mzone="Moscow"/> 
</timezone>

$ TZ=Europe/Minsk d8
d8> d1=new Date(2012,5,21)
Thu Jun 21 2012 00:00:00 GMT+0300 (Moscow Standard Time)  // should be Further-eastern Euroepean time
d8> d1.toLocaleString("en", {'timeZoneName': 'long'})
"6/21/2012, 12:00:00 AM Further-eastern European Time"
d8> d2=new Date(2018,5,21)
Thu Jun 21 2018 00:00:00 GMT+0300 (Moscow Standard Time)
d8> d2.toLocaleString("en", {'timeZoneName': 'long'})
"6/21/2018, 12:00:00 AM Moscow Standard Time"

A clearer example:   Europe/Minsk belongs to 'Moscow' metazone today, but it belonged to Eastern European metazone in 2005. In summer of 2005, Moscow Summer Time is UTC+4 while Eastern European Summer time was UTC+3. There's a mismatch between GMT offset outside parens and the timezone display name inside parens. 

d8> d6 = new Date(Date.UTC(2005, 5, 21, 12))
Tue Jun 21 2005 15:00:00 GMT+0300 (Moscow Summer Time)  // should be Eastern European Summer Time
d8> d6.toLocaleString("en", {'timeZoneName': 'long'})
"6/21/2005, 3:00:00 PM Eastern European Summer Time"
d8> d6.toLocaleString("en", {'timeZoneName':'long', 'timeZone': 'Europe/Moscow'})
"6/21/2005, 4:00:00 PM Moscow Summer Time"
d8> d6.toLocaleString("en", {'timeZoneName':'long', 'timeZone': 'Europe/Minsk'})
"6/21/2005, 3:00:00 PM Eastern European Summer Time"

re: comment 17 (1st issue in that comment) and comment 18

A temporary work-around in the ICU data would be to add 'Turkey Standard Time' (and 'Turkey Summer Time') in the root and Turkish corresponding entries in 'tr' locale for Turkey metazone. 
An alternative would be to do that in the v8 code, but it'd add a string comparison. 




A similar example to comment 20. Europe/Paris was UTC+1 year-round (no summer time) in 1969, but the zone name inside parens 'Central European Summer TIme' because it's cached. 

TZ=Europe/Paris $ d8
> new Date(2018,5,21).getTimezoneOffset()
-120
> new Date(2018,5,21) 
Thu Jun 21 2018 00:00:00 GMT+0200 (Central European Summer Time)
> new Date(1969, 5, 21).getTimezoneOffset()
-60
> new Date(1969, 5,21)
Sat Jun 21 1969 00:00:00 GMT+0100 (Central European Summer Time)

Wait. This is a different issue than what I wrote about in comment 20.  v8 should know that in June 1969, DST was not in effect and pick 'standard zone name' instead of 'summer timezone' name. 

I'll look into it in a separate bug. 

Sign in to add a comment