1172609 - Timezone settings retrieved by Intl are incorrect

Status: RESOLVED FIXED

(Core :: JavaScript: Internationalization API, defect)

Description

[Zibi Braniecki [:zbraniecki][:gandalf]]

There seem to be a bug in how we retrieve timezone settings from the system.

Steps to reproduce:

1) Test your setup:
var time1 = now.toLocaleFormat('%I:%M %p');
var offset = now.getTimezoneOffset();

In my setup, time1 returns '11:29 am', and the offset is 420 (-7 vs UTC);

2) Verify that manual setting works:

var time1 = now.toLocaleTimeString('en-US', {
  hour: 'numeric',
  minute: 'numeric',
  timeZone: 'UTC'
});

In my setup it returns '6:29 pm' which is correct

3) Try automatic:

var time2 = now.toLocaleTimeString('en-US', {
  hour: 'numeric',
  minute: 'numeric',
});


In my setup, on Desktop it returns '11:29 am', but on B2G it returns '6:29 pm' which indicates that Intl did not take the timezone.

================

There's one more bit:

var tf = new Intl.DateTimeFormat('en-US', {
  hour: 'numeric',
  minute: 'numeric',
});

tf.resolvedOptions();

returns an object with 'timeZone: undefined' on both, desktop and B2G, while in Chrome it returns properly 'America/Los_Angeles'

Comment 1

[Zibi Braniecki [:zbraniecki][:gandalf]]

Also, potentially related:

var tf = new Intl.DateTimeFormat(navigator.languages, {
      hour12: navigator.mozHour12,
      hour: 'numeric',
      minute: 'numeric',
      timeZone: 'America/Anchorage',
    });

errors both on Desktop and B2G with "RangeError: invalid time zone in DateTimeFormat(): AMERICA/ANCHORAGE"

Comment 2

[Jeff Walden [:Waldo]]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #1)
> Also, potentially related:
> 
> var tf = new Intl.DateTimeFormat(navigator.languages, {
>       hour12: navigator.mozHour12,
>       hour: 'numeric',
>       minute: 'numeric',
>       timeZone: 'America/Anchorage',
>     });
> 
> errors both on Desktop and B2G with "RangeError: invalid time zone in
> DateTimeFormat(): AMERICA/ANCHORAGE"

Support for time zones other than "UTC" is optional in the implementation.  We only support UTC right now, though at some point doubtless we'll go beyond that.

As to where ICU looks for the time zone, I don't know off the top of my head.  A source-skim *suggests* it looks at the TZ environment variable.  It creates a default time zone object that's initialized to that constant time zone.  It's not clear to me whether calling http://www.icu-project.org/apiref/icu4c/ucal_8h.html#a2544550264fccc52c97b53a2febf29cb will cause that to be refreshed or not.

The Date object's code for time zones, in contrast, looks at localtime, in realtime.  I don't know if localtime itself looks at TZ or not.

Comment 3

[Zibi Braniecki [:zbraniecki][:gandalf]]

So, :tedders found out that it is a bug in ICU and can be easily reproduced on desktop.

STR:

1) Look at the timezone in your OS
2) (new Date()).toLocaleFormat(); // local date
3) (new Date()).getTimezoneOffset(); // proper offset
4) (new Date()).toLocaleString(); // local date
5) Change date in your OS
6) (new Date()).toLocaleFormat(); // updated local date
7) (new Date()).getTimezoneOffset(); // new proper offset
8) (new Date()).toLocaleString(); // old local date

ICU does not update the timezone when it changes in OS

Comment 4

[Ted Clancy [:tedders1]]

Attachment: bug-1172609-icu-modifications.patch

This patch adds a new method to icu's TimeZone class called TimeZone::recreateDefault().

The code is a bit tricky, because it has to be threadsafe. As part of this code, I added support for recursive mutexes to ICU.

This code hasn't yet been tested, for reasons I'll explain in another comment.

Comment 5

[Ted Clancy [:tedders1]]

Hi Waldo,

I created a patch for the version of ICU that exists in intl/icu/source/, but our B2G build uses a different version of ICU in the external/icu4c/ directory. I'm not sure where external/icu4c/ is pulled from.

Do you know how I can patch the version of ICU in external/icu4c/ ?

Comment 6

[Ted Clancy [:tedders1]]

Wow, what idiot said this should be simple to fix. (That would be me.)

The problem turned out to be in ICU. ICU has no way to respond to timezone changes. It creates its "default timezone" object once, and has no way to change it. (There's an open bug about this on ICU's bug tracker.) So the only way to fix this is by patching ICU.

This is complicated by the fact that, as of Bug 866301 (Part 3), B2G no longer uses the in-tree version of ICU, and instead uses a version pulled from git://codeaurora.org/platform/external/icu4c , which I can't patch. So, to fix this, we'll have to start using the in-tree version again.

Do we really to do that? I say yes. But it's up to you guys.

This is going to be split over 7 patches. But, most of them are really small.

Comment 7

[Ted Clancy [:tedders1]]

Attachment: bug-1172609-icu-version.patch

This patch makes B2G use the in-tree version of ICU.

Comment 8

[Ted Clancy [:tedders1]]

Attachment: bug-1172609-icu-import.patch

This patch fixes ICU's import declaration when building shared libraries on GCC.

We don't actually need this patch, because we using static linking. But, for a while, I was looking at using a shared library, and this bug cost me a whole day. I'd like to fix it now before it causes someone else some trouble.

This bug exists on ICU's trunk and has been there for years. I don't understand how it's not causing more problems.

More info: https://gcc.gnu.org/wiki/Visibility

Comment 9

[Ted Clancy [:tedders1]]

Attachment: bug-1172609-icu-recursive-mutex.patch

This patch adds recursive mutex functionality to ICU.

This is for the benefit of AIX. According to comments in the ICU source code, AIX's tzset() is implemented using ICU. This means that when ICU calls tzset(), it can result in a recursive call to ICU. I added recursive mutex support so I can avoid deadlock in this scenario.

Note that we don't actually need this patch, since we aren't on AIX, but I'm including it for completeness.

Note also that I haven't actually tested this on AIX.

Comment 10

[Ted Clancy [:tedders1]]

Attachment: bug-1172609-icu-timezone-recreateDefault.patch

And here's where the magic happens.

This adds an API function to ICU called icu::TimeZone::recreateDefault().

This code is a little tricky because it has to be thread-safe.

Comment 11

[Ted Clancy [:tedders1]]

Attachment: bug-1172609-js-ResetTimeZone.patch

This patch adds js::ResetTimeZone().

I'm tagging Waldo to review the code in the js/src directory, and Ted (not me, other guy) to review the change to the file in the config directory.

Comment 12

[Ted Clancy [:tedders1]]

Attachment: bug-1172609-nsjsutils-ResetTimeZone.patch

This patch adds nsJSUtils::ResetTimeZone().

Comment 13

[Ted Clancy [:tedders1]]

Attachment: bug-1172609-hal-calls-ResetTimeZone.patch

This patch makes the hardware abstraction layer call nsJSUtils::ResetTimeZone() when the timezone changes.

Comment 14

[Ted Clancy [:tedders1]]

Oh, did I mention that this only fixes the problem on B2G?

It doesn't fix the problem on desktop or Android, because those platforms don't have a moztimechange event that we can hook into. One would have to be added.

Comment 15

[Ted Clancy [:tedders1]]

Attachment: bug-1172609-hal-calls-ResetTimeZone.patch

(Fixing commit comment.)

Comment 16

[Ted Clancy [:tedders1]]

Treeherder results: https://treeherder.mozilla.org/#/jobs?repo=try&revision=20536f491266

Comment 17

[Zibi Braniecki [:zbraniecki][:gandalf]]

Simon, Jeff: what's the ETA for this review? It's blocking us from replacing l10n_date shim with Intl in Firefox OS and that impacts the whole L10n/Intl integration into Gaia's NG architecture.

Comment 18

[Simon Montagu :smontagu]

Comment on attachment 8622333 [details] [diff] [review]
bug-1172609-icu-import.patch

Review of attachment 8622333 [details] [diff] [review]:
-----------------------------------------------------------------

I've never been involved with patching our in-tree ICU so I'm going to punt to Waldo on all these requests.

Comment 19

[(not currently active) Ted Mielczarek]

Comment on attachment 8622331 [details] [diff] [review]
bug-1172609-icu-version.patch

Review of attachment 8622331 [details] [diff] [review]:
-----------------------------------------------------------------

I think I'd want Waldo to sign off on switching back to the in-tree ICU on B2G, but otherwise this is fine with me.

Comment 20

[(not currently active) Ted Mielczarek]

Comment on attachment 8622343 [details] [diff] [review]
bug-1172609-js-ResetTimeZone.patch

Review of attachment 8622343 [details] [diff] [review]:
-----------------------------------------------------------------

I don't think there's really anything for me to review in this patch, check_spidermonkey_style.py isn't really part of the build system.

Comment 21

[Zibi Braniecki [:zbraniecki][:gandalf]]

Ted, if I remember correctly you asked me to ping you when ICU upgrade lands in case you'd need to update the patches. 

Bug 1075758, which updates ICU to 55.1, landed.

Comment 22

[Ted Clancy [:tedders1]]

Attachment: bug-1172609-icu-timezone-recreateDefault.patch

Rebased against new in-tree ICU.

I haven't had a chance to test this yet. Gandalf, can you test this again?

Comment 23

[Jeff Walden [:Waldo]]

Comment on attachment 8622343 [details] [diff] [review]
bug-1172609-js-ResetTimeZone.patch

Review of attachment 8622343 [details] [diff] [review]:
-----------------------------------------------------------------

In principle this is okay.  Still need to see the rest of the patches before I say confidently we want this.

::: js/src/jsapi.cpp
@@ +6048,5 @@
>      return obj->zone();
>  }
> +
> +JS_PUBLIC_API(void)
> +JS::ResetTimeZone()

This should be in jsdate.cpp.

::: js/src/jsapi.h
@@ +37,5 @@
>  
>  namespace JS {
>  
> +extern JS_PUBLIC_API(void)
> +ResetTimeZone();

Put this in js/public/Date.h at the end, and add a /** */ documentation comment explaining what it does.

Comment 24

[Jeff Walden [:Waldo]]

Comment on attachment 8622333 [details] [diff] [review]
bug-1172609-icu-import.patch

Review of attachment 8622333 [details] [diff] [review]:
-----------------------------------------------------------------

Please file an ICU bug with this patch: http://bugs.icu-project.org/trac/newticket  We've generally hewn fairly closely to having all our local patches correspond to upstream ICU issues, when the patch makes sense upstream, as this one certainly does.

Also, changes to ICU files require that you dump a patch file into intl/icu-patches, then apply that patch in intl/update-icu.sh.  It should be the case that if you run intl/update-icu.sh with the URL in the comment about usage in that script, it changes nothing in your tree.

::: intl/icu/source/common/unicode/platform.h
@@ +731,3 @@
>  #elif defined(_MSC_VER)
>      /* Windows needs to export/import data. */
>  #   define U_IMPORT __declspec(dllimport)

I suspect you should put your #elif after the _MSC_VER test.  For clang on Windows, we almost certainly want to use the declspec stuff, and I don't know that clang on Windows doesn't define __GNUC__ there.

Comment 25

[Jeff Walden [:Waldo]]

Comment on attachment 8622338 [details] [diff] [review]
bug-1172609-icu-recursive-mutex.patch

Review of attachment 8622338 [details] [diff] [review]:
-----------------------------------------------------------------

Again, this needs an upstream bug, and intl/update-icu.sh and intl/icu-patches modifications.  Given this is code we don't actually use, I don't mind taking this patch, for sure.  Although I guess us taking it in that situation is rather odd.  :-)

Comment 26

[Jeff Walden [:Waldo]]

Comment on attachment 8622340 [details] [diff] [review]
bug-1172609-icu-timezone-recreateDefault.patch

Review of attachment 8622340 [details] [diff] [review]:
-----------------------------------------------------------------

So, uh.  Yeah.  This all seems rather hairy and fragile, and certainly not the sort of thing I really want us carrying around on our own.  If we even can -- we use system ICU on b2g some places because we didn't want the hit of two separate ICU versions on devices.  Please file an upstream bug, or attach this patch to an existing upstream bug, and get feedback on it there.  I don't think this is something we should be taking, with a bunch of mutex hair, unless upstream has okayed a patch.

On the topic of the code, I think you should add a bit more encapsulation here.  All this default time zone stuff should be encapsulated in a class, having a single static instance.  The one or two public methods can call private methods to acquire the mutex, in ways that guarantee the mutex is held when critical sections are entered.  In this patch it's not necessarily that obvious, IMO.  Names like "initMutex" seem very general and would leave me with little understanding of when they are/should be called, whereas making them private to a class cabins things much better.

Comment 27

[Jeff Walden [:Waldo]]

Comment on attachment 8631997 [details] [diff] [review]
bug-1172609-icu-timezone-recreateDefault.patch

Review of attachment 8631997 [details] [diff] [review]:
-----------------------------------------------------------------

This looks somewhat basically the same as the other patch to me.  Upstream whichever is the latest one, of course (although I do think the encapsulation I mentioned in the last comment should be done before that happens).

Comment 28

[Jeff Walden [:Waldo]]

Oh, switching back to in-tree ICU is a B2G owner decision, not on me.  I am absolutely fine with using just the in-tree thing, as it'd simplify our lives a bunch to have exactly the one thing we use available for debugging, the rare bit of patching, and to let us remove dependencies on unstable ICU C++ APIs.  (There's at least one I'll remove once we can depend on ICU > 52 everywhere.)  But I can't force that decision in B2G-land, you need someone like fabrice or so to give the okay, I think.

Comment 29

[Zibi Braniecki [:zbraniecki][:gandalf]]

Jonas, would you be ok with us switching to in-tree ICU in order to fix this bug and be able to migrate to Intl API?

Comment 30

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I don't know enough about this to have an opinion. I'd defer to Ehsan who I think knows more about our ICU libraries.

Comment 31

[Zibi Braniecki [:zbraniecki][:gandalf]]

Ehsan, can you help us with that? We'd need a confirmation that we can switch to in-tree ICU so that we can patch it locally in order to be able to migrate to use Intl API for Firefox OS 2.5.

Comment 32

[(no longer active)]

The only reason I know of why we may not want to do that is that we would have two copies of ICU on the device (one the in-tree libraries, and the other that we inherit from the AOSP bits) and in the past this used to be an issue because we wanted to support devices such as Tarako.  As that doesn't seem to be a concern, I don't know of any other reason why we cannot switch to the in-tree ICU, so let's do that!

Comment 33

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to Ehsan Akhgari (not reviewing patches, not reading bugmail, needinfo? me!) from comment #32)
> The only reason I know of why we may not want to do that is that we would
> have two copies of ICU on the device (one the in-tree libraries, and the
> other that we inherit from the AOSP bits) and in the past this used to be an
> issue because we wanted to support devices such as Tarako.  As that doesn't
> seem to be a concern, I don't know of any other reason why we cannot switch
> to the in-tree ICU, so let's do that!

We need before and after memory measurements here.

Comment 34

[(no longer active)]

Yes, good idea.

Comment 35

[Zibi Braniecki [:zbraniecki][:gandalf]]

Ted, we decided to not block landing of Intl on this regression because it has been blocking an increasing pile of work and with Ehsan and Waldo sr approvals of the shift to in-tree ICU with a patch it seems that we can resolve it, but I'd still prefer to keep the time when we are working with the regression as small as possible.

Ted, do you have any ETA for this bug?

Comment 36

[Ted Clancy [:tedders1]]

> Ted, do you have any ETA for this bug?

Tuesday.

Comment 37

[Ted Clancy [:tedders1]]

Attachment: bug-1172609-js-ResetTimeZone.patch

Comment 38

[Ted Clancy [:tedders1]]

Attachment: bug-1172609-icu-timezone-detection.patch

Comment 39

[Jeff Walden [:Waldo]]

Comment on attachment 8641256 [details] [diff] [review]
bug-1172609-icu-timezone-detection.patch

Review of attachment 8641256 [details] [diff] [review]:
-----------------------------------------------------------------

In principle this doesn't look too bad.  But is it complete?  As I look at uprv_tzname, I see it's controlled by the gTimeZoneBufferPtr stuff that this patch resets -- but only #if U_PLATFORM_USES_ONLY_WIN32_API and #ifndef DEBUG_TZNAME.  The former is fine, we never define it so wouldn't hit that code.  But the latter case, I'm pretty sure that holds.  So with this patch, it would appear we'll consult the TZ environment variable, and if that's isValidOlsonID(tzid), this patch have any effect.  And it seems like TZ will quite often be set, such that this patch would appear to have no effect.

Or am I missing something here?

Comment 40

[Ted Clancy [:tedders1]]

Oops, sorry, Waldo. I flagged you on that prematurely.

Comment 42

[Ted Clancy [:tedders1]]

Attachment: bug-1172609-icu-timezone-recreateDefault.patch

Comment 43

[Ted Clancy [:tedders1]]

Attachment: bug-1172609-icu-timezone-detection.patch

Comment 44

[Ted Clancy [:tedders1]]

Attachment: bug-1172609-icu-timezone-recreateDefault.patch

Comment 45

[Ted Clancy [:tedders1]]

Green try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e05be9625bee

Comment 46

[Ted Clancy [:tedders1]]

Using the STR in Comment 3, I've confirmed that this works.

Comment 47

[Ted Clancy [:tedders1]]

(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #39)

> As I look at
> uprv_tzname, I see it's controlled by the gTimeZoneBufferPtr stuff that this
> patch resets -- but only #if U_PLATFORM_USES_ONLY_WIN32_API and #ifndef
> DEBUG_TZNAME.

I think you might be looking at the wrong place. The code depends on CHECK_LOCALTIME_LINK, which is an existing macro which is defined earlier in the source file for Linux-like platforms.

Comment 48

[Michael Henretty [:mikehenrty][:mhenretty]]

Ted, do you know if there is anyone else who can review this code? Jeff Walden might be swamped with other stuff, and this patch solves a lot of headaches for our current Foxfooders.

Comment 49

[Zibi Braniecki [:zbraniecki][:gandalf]]

[Blocking Requested - why for this release]: We're switching all Gaia apps to Intl API which gives us performance and memory wins and is a requirement for NGA L10n API.

This is the only regression we encountered so far after switching LockScreen, Statusbar, Settings and SMS.

If we won't get this landed soon, we will have to start reverting all those changes and that would be a mountain of work and bad for 2.5 QA.

Comment 50

[Jeff Walden [:Waldo]]

Comment on attachment 8644090 [details] [diff] [review]
bug-1172609-icu-timezone-detection.patch

Review of attachment 8644090 [details] [diff] [review]:
-----------------------------------------------------------------

Regarding comment 47, what I mean is that in order to reach the gTimeZoneBufferPtr setting code, you have have *not* exited anywhere earlier.  But if I look at uprv_tzname, I see this at the start:

///////////////////////////////////////////////////////////////////////////////
U_CAPI const char* U_EXPORT2
uprv_tzname(int n)
{
    const char *tzid = NULL;
#if U_PLATFORM_USES_ONLY_WIN32_API
    tzid = uprv_detectWindowsTimeZone();

    if (tzid != NULL) {
        return tzid;
    }
#else

/*#if U_PLATFORM_IS_DARWIN_BASED [...] #endif */

/* This code can be temporarily disabled to test tzname resolution later on. */
#ifndef DEBUG_TZNAME
    tzid = getenv("TZ");
    if (tzid != NULL && isValidOlsonID(tzid)
#if U_PLATFORM == U_PF_SOLARIS
    /* When TZ equals localtime on Solaris, check the /etc/localtime file. */
        && uprv_strcmp(tzid, TZ_ENV_CHECK) != 0
#endif
    ) {
        /* This might be a good Olson ID. */
        skipZoneIDPrefix(&tzid);
        return tzid;
    }
    /* else U_TZNAME will give a better result. */
#endif

#if defined(CHECK_LOCALTIME_LINK) && !defined(DEBUG_SKIP_LOCALTIME_LINK)
    /* Caller must handle threading issues */
    if (gTimeZoneBufferPtr == NULL) {
///////////////////////////////////////////////////////////////////////////////

In order to reach that code at the end, the earlier returns can't have been hit.

The second case definitely is live code, and it looks pretty clearly like it can be hit.  DEBUG_TZNAME is only mentioned in this file, and it's never defined, so this code will run.  Example:

[jwalden@find-waldo-now src]$ TZ=America/New_York dbg/js/src/js -e 'print(new Intl.DateTimeFormat("en-US", { month: "2-digit", day: "2-digit", year: "2-digit", hour: "2-digit", minute: "2-digit", second: "2-digit" }).format(Date.now()));'
08/21/15, 6:53:35 PM
[jwalden@find-waldo-now src]$ TZ=America/Los_Angeles dbg/js/src/js -e 'print(new Intl.DateTimeFormat("en-US", { month: "2-digit", day: "2-digit", year: "2-digit", hour: "2-digit", minute: "2-digit", second: "2-digit" }).format(Date.now()));'
08/21/15, 3:53:38 PM
[jwalden@find-waldo-now src]$ dbg/js/src/js -e 'print(new Intl.DateTimeFormat("en-US", { month: "2-digit", day: "2-digit", year: "2-digit", hour: "2-digit", minute: "2-digit", second: "2-digit" }).format(Date.now()));'
08/21/15, 3:53:43 PM

When TZ is set to a valid Olson ID, your patch won't have any effect, because the gTimeZoneBufferPtr code will never be reached.  On my system TZ happens not to be set, but *in general* you don't know that it won't be.  What are we going to do to handle that?

The first case, U_PLATFORM_USES_ONLY_WIN32_API, is less clearly going to be hit.  But if I do a basic Windows shell build and step through the code, I see that we call uprv_detectWindowsTimeZone(), it returns "America/Los_Angeles" for me, and this resetting code would never come into play.  Now, maybe the Windows API *does* get the time zone change handling correct already.  But certainly there's no explanation/proof in this bug of why that would be true.

All of which is to say, these patches aren't exactly *wrong*, but in two separate cases these patches will do exactly nothing.  The Windows case may well affect all Windows machines -- my cursory investigation of where U_PLATFORM_USES_ONLY_WIN32_API is set suggests all our Windows builds might be affected.  And the TZ possibility affects an unknown number of *nix machines that might maintain TZ to some static value.  I could accept these patches, but these two cases must be resolved before this bug is acceptably fixed.

Comment 51

[Ted Clancy [:tedders1]]

Hi Waldo,

As I mentioned in Comment 14, this fix was only for B2G. This fix won't work on Windows or Mac, since those platforms don't have the moztimechange event (which this fix relies on).

But thank you for pointing out that the ICU patch isn't useful on Windows or Mac.

Perhaps a bug should be filed for this issue on Windows/Mac/Android platforms.

Comment 52

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/858873762b60
https://hg.mozilla.org/integration/mozilla-inbound/rev/3be69e9052d4
https://hg.mozilla.org/integration/mozilla-inbound/rev/382365abacde
https://hg.mozilla.org/integration/mozilla-inbound/rev/811e0e236714
https://hg.mozilla.org/integration/mozilla-inbound/rev/37d99d80d58b
https://hg.mozilla.org/integration/mozilla-inbound/rev/b1255b21a94a

Comment 53

[Gregor Wagner [:gwagner]]

Zibi, can you verify the use-cases from the description?

Comment 54

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/858873762b60
https://hg.mozilla.org/mozilla-central/rev/3be69e9052d4
https://hg.mozilla.org/mozilla-central/rev/382365abacde
https://hg.mozilla.org/mozilla-central/rev/811e0e236714
https://hg.mozilla.org/mozilla-central/rev/37d99d80d58b
https://hg.mozilla.org/mozilla-central/rev/b1255b21a94a

Comment 55

[Zibi Braniecki [:zbraniecki][:gandalf]]

(In reply to Gregor Wagner [:gwagner] from comment #53)
> Zibi, can you verify the use-cases from the description?

It mostly does. Our primary use case works properly now, but I see one edge case scenario which may require additional fix.

Basically, when I pass first parameter to toLocaleString it does use the current timezone properly, but if I omit the first parameter, it does use the old timezone.

Steps to reproduce:

1) Set a timezone on your device to America/Los Angeles
2) Restart a phone
3) (new Date()).toLocaleString(navigator.languages); // proper local datetime with America/Los Angeles timezone
4) (new Date()).toLocaleString(); // proper local datetime with America/Los Angeles timezone
5) Change time zone to America/New York
6) (new Date()).toLocaleString(navigator.languages); // proper local datetime with America/New York timezone
7) (new Date()).toLocaleString(); // old datetime with America/Los Angeles timezone

Expected result:

In setp 7) the datetime should use America/New York

:tedders, does it sound like a bug to you or is it per design?

Comment 56

[Jeff Walden [:Waldo]]

That's an artifact of js/src/builtin/Date.js lines 34-35 having a not-really-principled cache for the formatter to use with toLocaleString() without arguments.  It could be eliminated to make things live.  If it had to survive in some form, however...well, pfui.  Somehow you need to be able to ask ICU what the *used* time zone was for a UDateFormat.  I can't figure out how from my chickenscratch ICU header-reading, but that doesn't mean it doesn't exist.  Would have to read harder to figure it out, if it were necessary.

Comment 57

[Jeff Walden [:Waldo]]

I filed bug 1205586 with a patch for the issue discussed in comment 55 and comment 56.  Once that lands I'll probably try to grab someone b2g-ish to verify the fix works, so some people here might be getting a low-priority poke (not here) for that.

Comment 58

[Jeff Walden [:Waldo]]

Oh, bah, I forgot ICU consults a different thing for the time zone than the rest of the JS engine, as I pointed in comment 56.  Never mind on poking about testing the fix there.