Timezone settings retrieved by Intl are incorrect

RESOLVED FIXED in Firefox 43

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: zbraniecki, Assigned: tedders1)

Tracking

(Blocks 1 bug)

unspecified
mozilla43
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.5+, firefox43 fixed, b2g-master fixed)

Details

(Whiteboard: [systemsfe])

Attachments

(6 attachments, 9 obsolete attachments)

1.64 KB, patch
ted
: review+
Details | Diff | Splinter Review
2.48 KB, patch
Waldo
: review+
Details | Diff | Splinter Review
1.89 KB, patch
Ehsan
: review+
Details | Diff | Splinter Review
1.36 KB, patch
dhylands
: review+
Details | Diff | Splinter Review
6.41 KB, patch
Waldo
: review+
Details | Diff | Splinter Review
7.53 KB, patch
Waldo
: review+
Details | Diff | Splinter Review
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'
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"
(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.
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
Component: Gaia::System → JavaScript: Internationalization API
Product: Firefox OS → Core
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.
Assignee: nobody → tclancy
Attachment #8621091 - Flags: review?(jwalden+bmo)
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/ ?
Flags: needinfo?(jwalden+bmo)
Target Milestone: --- → 2.2 S14 (12june)
Attachment #8621091 - Attachment is obsolete: true
Attachment #8621091 - Flags: review?(jwalden+bmo)
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.
Flags: needinfo?(jwalden+bmo)
This patch makes B2G use the in-tree version of ICU.
Attachment #8622331 - Flags: review?(ted)
Posted patch bug-1172609-icu-import.patch (obsolete) — Splinter Review
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
Attachment #8622333 - Flags: review?(smontagu)
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.
Attachment #8622338 - Flags: review?(smontagu)
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.
Attachment #8622340 - Flags: review?(smontagu)
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.
Attachment #8622343 - Flags: review?(ted)
Attachment #8622343 - Flags: review?(jwalden+bmo)
This patch adds nsJSUtils::ResetTimeZone().
Attachment #8622345 - Flags: review?(ehsan)
This patch makes the hardware abstraction layer call nsJSUtils::ResetTimeZone() when the timezone changes.
Attachment #8622346 - Flags: review?(dhylands)
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.
(Fixing commit comment.)
Attachment #8622346 - Attachment is obsolete: true
Attachment #8622346 - Flags: review?(dhylands)
Attachment #8622350 - Flags: review?(dhylands)
Attachment #8622345 - Flags: review?(ehsan) → review+
Attachment #8622350 - Flags: review?(dhylands) → review+
See Also: → 1075758
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.
Flags: needinfo?(smontagu)
Flags: needinfo?(jwalden+bmo)
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.
Attachment #8622333 - Flags: review?(smontagu) → review?(jwalden+bmo)
Flags: needinfo?(smontagu)
Attachment #8622338 - Flags: review?(smontagu) → review?(jwalden+bmo)
Attachment #8622340 - Flags: review?(smontagu) → review?(jwalden+bmo)
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.
Attachment #8622331 - Flags: review?(ted) → review+
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.
Attachment #8622343 - Flags: review?(ted)
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.
Flags: needinfo?(tclancy)
Whiteboard: [systemsfe]
Rebased against new in-tree ICU.

I haven't had a chance to test this yet. Gandalf, can you test this again?
Flags: needinfo?(tclancy)
Attachment #8631997 - Flags: review?(smontagu)
Attachment #8631997 - Flags: review?(smontagu) → review?(jwalden+bmo)
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.
Attachment #8622343 - Flags: review?(jwalden+bmo) → review+
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.
Attachment #8622333 - Flags: review?(jwalden+bmo) → feedback+
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.  :-)
Attachment #8622338 - Flags: review?(jwalden+bmo) → feedback+
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.
Attachment #8622340 - Flags: review?(jwalden+bmo)
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).
Attachment #8631997 - Flags: review?(jwalden+bmo)
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.
Flags: needinfo?(jwalden+bmo)
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?
Flags: needinfo?(jonas)
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.
Flags: needinfo?(jonas)
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.
Flags: needinfo?(ehsan)
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!
Flags: needinfo?(ehsan)
(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.
Yes, good idea.
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?
Flags: needinfo?(tclancy)
> Ted, do you have any ETA for this bug?

Tuesday.
Flags: needinfo?(tclancy)
Attachment #8622333 - Attachment is obsolete: true
Attachment #8622338 - Attachment is obsolete: true
Attachment #8622340 - Attachment is obsolete: true
Attachment #8631997 - Attachment is obsolete: true
Attachment #8622343 - Attachment is obsolete: true
Attachment #8641256 - Flags: review?(jwalden+bmo)
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?
Flags: needinfo?(tclancy)
Oops, sorry, Waldo. I flagged you on that prematurely.
Flags: needinfo?(tclancy)
Attachment #8641254 - Attachment is obsolete: true
Attachment #8622343 - Attachment is obsolete: false
Duplicate of this bug: 1188801
Attachment #8641256 - Flags: review?(jwalden+bmo)
Attachment #8643822 - Flags: review?(jwalden+bmo)
Attachment #8641256 - Flags: review?(jwalden+bmo)
Attachment #8643822 - Attachment is obsolete: true
Attachment #8643822 - Flags: review?(jwalden+bmo)
Attachment #8641256 - Attachment is obsolete: true
Attachment #8641256 - Flags: review?(jwalden+bmo)
Attachment #8644090 - Flags: review?(jwalden+bmo)
Using the STR in Comment 3, I've confirmed that this works.
(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.
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.
Flags: needinfo?(tclancy)
[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.
blocking-b2g: --- → 2.5?
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.
Attachment #8644090 - Flags: review?(jwalden+bmo) → review+
Attachment #8644091 - Flags: review?(jwalden+bmo) → review+
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.
Flags: needinfo?(tclancy)
Keywords: checkin-needed
Zibi, can you verify the use-cases from the description?
Flags: needinfo?(gandalf)
blocking-b2g: 2.5? → 2.5+
Depends on: 1198216
Depends on: 1198237
(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?
Flags: needinfo?(gandalf) → needinfo?(tclancy)
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.
Flags: needinfo?(tclancy)
Depends on: 1198952
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.
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.
See Also: → 1208808
Depends on: 1218027
You need to log in before you can comment on or make changes to this bug.