> I'm proposing in the ICU bug that ICU check the content of /etc/timezone before /etc/localtime symlink target path. 
 

Hmm... /etc/timezone seems less portable(?) than /etc/localtime?  Nonetheless, trying it before following the symlink target for /etc/localtime might be better. 

From Chrome's timezone monitoring code: 

 // There is no true standard for where time zone information is actually
    // stored. glibc uses /etc/localtime, uClibc uses /etc/TZ, and some older
    // systems store the name of the time zone file within /usr/share/zoneinfo
    // in /etc/timezone. Different libraries and custom builds may mean that
    // still more paths are used. Just watch all three of these paths, because
    // false positives are harmless, assuming the false positive rate is
    // reasonable.

According to https://ssl.icu-project.org/trac/ticket/12770 :  RedHat and SuSe Linux also began to use a relative path ( ../usr/share/zoneinfo ). 

It looks like I have to apply a local patch. 

macOS 13.x (High Sierra : to be released soon) has a similar issue. Its zonefiles are in /var/db/.../zoneinfo instead of /usr/share/zoneinfo. 

Yes, it looks like this is changing in 10.13, but we don’t use time_zone_monitor_linux.cc on macOS, we use time_zone_monitor_mac.mm, and it shouldn’t care about the paths. Also, these paths don’t have dots in them, which is the part that you’re saying confuses ICU.

On the other hand, we do have sandbox exceptions to allow child processes to access timezone definitions, and I believe we’ll need to expand those. I filed  bug 754280  for that aspect.

Thanks, Mark !

For the record, https://ssl.icu-project.org/trac/ticket/13291 is an ICU bug for macOS 10.13. 

OK. I’m surprised this causes a bug for ICU, since as I showed in  bug 754280 , there’s a symbolic link at /usr/share/zoneinfo pointing to the new location.

$ ls -l /etc/localtime
lrwxr-xr-x  1 root  wheel  29 Aug  7 18:55 /etc/localtime -> /var/db/timezone/zoneinfo/UTC

ICU does a hack of extracting the Olson timezone ID from the symlink target ( /var/db/timezone/zoneinfo/<Olson ID path> on macOS 10.13 and /usr/share/zoneinfo/<Olson ID path> on macOS 10.12 or earlier) by chopping off the hardcoded path prefix ( '/usr/share/zoneinfo'). 

If the above fails (which will on macOS 10.13), it falls back to comparing the file content (resolved file content) of /etc/localtime against the content of each zone file scanning zonefiles under  /usr/share/zoneinfo (which is symlinked to /var/db/zoneinfo ). Often this does not work very well as shown in the ICU bug by pedberg, it returns 'US/Eastern' instead of 'America/New_York' because 'US/Eastern' happens to be scanned before 'America/New_York' and their contents are identical. This is not terrible but returning 'Europe/Isle_of_Mann' when the timezone is set to 'Europe/London' is worse. 

$ cd /usr/share/zoneinfo/Europe/
$ diff Isle_of_Man London
$ cd ..
$ diff America/New_York  US/Eastern
$ cd 

I'm proposing to Fuschia to have an API to return the Olson ID for the current timezone. A new OS can do better than old ones :-).   Well, old ones can be improved, too. 

I see. That’s a little gross. It sounds like the TZZONEINFO2 thing you suggest in https://ssl.icu-project.org/trac/ticket/13291#comment:4 would take care of it.

I like to take the patch there (if the upstream does not take it soon), but then I have to ask the patch author to upload it to chromiumreview.... and sign CHromium CLA. That can be done, but it's a bit more hassle for the patch author. So, I'm waiting for the upstream to take it (after asking the author to sign the Unicode CLA for ICU). 

https://chromium-review.googlesource.com/c/chromium/deps/icu/+/710496 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/deps/icu.git/+/21d33b1a09a77f033478ea4ffffb61e6970f83bd

commit 21d33b1a09a77f033478ea4ffffb61e6970f83bd
Author: Jungshik Shin <jshin@chromium.org>
Date: Wed Oct 11 18:45:03 2017

Fix timezone detection on macOS 10.13/newer Linux distros

The location of zoneinfo directory has changed on macOS 10.13,
Ubuntu 16, RHEL 7 and SuSe Linux 12. It results in the misdetection
of the OS timezone by ICU. Cherry-picking the CLs for the following
upstream bug fixes it.

https://ssl.icu-project.org/trac/ticket/12770

Bug:754053, 766916 
Test: In Javascript console, the following should give the correct
   timezone.
    (new Intl.DateTimeFormat()).resolvedOptions().timeZone

Change-Id: I711f4b27e73dc6855951055a601e80f711d34423
Reviewed-on: https://chromium-review.googlesource.com/710496
Reviewed-by: Mark Mentovai <mark@chromium.org>

[modify] https://crrev.com/21d33b1a09a77f033478ea4ffffb61e6970f83bd/README.chromium
[add] https://crrev.com/21d33b1a09a77f033478ea4ffffb61e6970f83bd/patches/timezone_detection.patch
[modify] https://crrev.com/21d33b1a09a77f033478ea4ffffb61e6970f83bd/source/common/putil.cpp

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1b4f3c6bdfaf9fd610d2dabf11a686fd1df56951

commit 1b4f3c6bdfaf9fd610d2dabf11a686fd1df56951
Author: Jungshik Shin <jshin@chromium.org>
Date: Thu Oct 12 01:18:14 2017

Roll ICU to 21d33b1

This has one change to fix the OS timezone detection on macOS 10.13
and newer Linux distros such as Ubuntu 16, RHEL 7, SuSe 12 or newer.

 https://chromium.googlesource.com/chromium/deps/icu/+log/7f873c45..21d33b1a

    (new Intl.DateTimeFormat()).resolvedOptions().timeZone
TBR=mark@chromium.org

Bug:  754053 , 766916 
Test: In Javascript console, the following should give the correct timezone.
Change-Id: I9553905fb20f0bf0b99442b38b2e67082c6fc64d
Reviewed-on: https://chromium-review.googlesource.com/713959
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Jungshik Shin <jshin@chromium.org>
Commit-Queue: Jungshik Shin <jshin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508199}
[modify] https://crrev.com/1b4f3c6bdfaf9fd610d2dabf11a686fd1df56951/DEPS

Nominating for M62. macOS 10.13 is out and the change in comment 14 is very safe.  
Actual change requires rolling ICU to 21d33b1 . 

This bug requires manual review: We are only 4 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

What are the downsides of waiting for this until M63? We are few days away form M62 stable, and this doesn't appear to be a critical change. Rejecting merge for now, but if you think this should be re-considered, please provide a detailed justification for why this is critical for M62. 

The downside of not merging  is that v8's Intl.DateTimeFormat() would malfunction on macOS 10.13 (well, it has been on Ubuntu 16/RHEL 7/SuSe 12 or newer, which does not help my argument ;-), but I can argue that Linux users are small compared with macOS users..) until 63 turns stable. That is, those who upgraded macOS to 10.13 will suddenly begin to see some web sites (using Intl.DateTimeFormat API) display the time incorrectly. BTW, this was fixed in Firefox in their latest release. 


The corresponding change for v8's non-Intl API (Date.*) was fixed in bug 713404. The fix for that is in M62 (it's done early enough. I should have picked up the upstream fix sooner; comment 11). 

So, there's a disparity between non-Intl API and Intl API. 

Because EcmaScript's Intl API offers better ways to display date/time,  we've been encouraging developers to use it (AngularJS uses it). 

Would this be good enough? 

Thanks for more feedback. Can you confirm if this is well tested in Canary or Dev? 

I've just tested the following version, on macOS 10.13 (I've just upgraded my MBA to that version), to confirm that it's fixed. 

Version 64.0.3240.0 (Official Build) canary (64-bit)

BTW, in addition to an ICU roll, the fix for  bug 773532  is  necessary in case of macOS. So, both the ICU roll and the fix for  bug 773532  have to be merged to M62 branch. 

Just in case, I also tried 64.0.3241.0 canary build on macOS 10.12 (not 10.13). It worked fine there, too.

A bit of the clarification:  the actual change is only the following:

https://chromium-review.googlesource.com/c/chromium/deps/icu/+/710496/4/source/common/putil.cpp

 

This bug requires manual review: We are only 1 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Adding  bug 773532  as a blocker to make clear that  bug 773532  fix is also necessary for macOS. 

I verified that it's fixed on macOS 10.13 with the latest canary build. The same should be the case of Ubuntu 16.x/17.x and RHEL and SuSe Linux. 

Note that this bug is specifically about Intl.DateTimeFormat().  Date.toString() issue is still outstanding on macOS 10.13 (High Sierra) and it's  bug 773532  .

Approving merge to M62 based on comments 25,22,19. Branch:3202

Thank you for the approval. 

A little bit more details. 

* Intl.DateTimeFormat and Date.toLocaleString() 

   - Linux (Ubuntu 16/17, RHEL 7, etc): Fixed with the ICU fix recorded here alone
   - macOS 10.12: a sandbox hole change in  bug 773532  fixed the issue. The ICU fix is not necessary
   - macOS 10.13: The ICU fix here + the sandbox hole change in  bug 773532  fixed completely.

* Date.toString() and Date.getTimezoneOffset()
   - Linux: Fixed with the ICU fix here +  https://chromium-review.googlesource.com/c/v8/v8/+/713404

    - macOS 10.12: completely fixed with the fix for  bug 773532 
    - macOS 10.13: partly fixed with the ICU fix here + fix for  bug 773532  - an additional sandbox hole is necessary in  bug 773532  . 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/tools/buildspec/+/15e55ed197f16bcb3e22467bdb0e9b389221e46b

commit 15e55ed197f16bcb3e22467bdb0e9b389221e46b
Author: Jungshik Shin <jungshik@google.com>
Date: Mon Oct 16 18:19:42 2017

marking it with merged label, since this merged.