1501875 - new Date().toString() emits localized, long timezone on Firefox 63, at least 1 site is broken

Status: RESOLVED WONTFIX

(Core :: JavaScript: Standard Library, defect)

Description

[Kohei Yoshino]

A Japanese Firefox user has reported that Japanese stock trading site kabu.com is broken on Firefox 63, the prices are not updated properly. The site requires an account, but he could send me the error console message and their publicly accessible source code.

https://twitter.com/yassan138/status/1054994628087345159
https://q20.tfx.kabu.co.jp/condor-web-kbc-tfxcfd/_js/ajax.js 36:1

> TypeError: Cannot convert string to ByteString because the character at index 35 has value 26085 which is greater than 255.

It's an Ajax code and a minimum testcase would look like this:

> const xhr = new XMLHttpRequest();
> xhr.open('GET', '');
> xhr.setRequestHeader('If-Modified-Since', new String(new Date()));

The problem here is, the timezone in the stringified date is localized on Firefox 63. The results of `new Date().toString()` will be:

Firefox 62 Japanese (ja):

> "Wed Oct 24 2018 19:00:54 GMT-0400 (EDT)"

Firefox 63 Japanese (ja):

> "Wed Oct 24 2018 19:00:44 GMT-0400 (アメリカ東部夏時間)"

Firefox 63 Canadian English (en-CA):

> "Wed Oct 24 2018 19:00:28 GMT-0400 (Eastern Daylight Time)"

Non-ASCII characters cannot be contained in an HTTP header, hence the TypeError. The timezone format should be the same as before.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/toString

[Tracking Requested - why for this release]: A site compatibility regression affecting a real world website.

Comment 1

[Kohei Yoshino]

The same result with Firefox 63 American English (en-US):

> "Wed Oct 24 2018 19:23:48 GMT-0400 (Eastern Daylight Time)"

Comment 2

[Jeff Walden [:Waldo]]

DateTimeHelper::timeZoneComment when we're compiling with ICU, as of bug 1346211, will put in a time zone display name per what icu::TimeZone indicates.  Before then, we would consult PRMJTime goo and get back a time zone, then we'd filter out any time zone that was '()' or had non-ASCII characters in it.

We could start doing some sort of filtering vaguely like that again if we wanted.  Or we could cons up a time zone identifier purely from the offset, like GMT+08:00 or so, and avoid consulting ICU for this at all, maybe.  The spec requires this bit of string to be as computed by a TimeZoneString function in the spec, so if we change stuff, we do need to be careful to change it for all such users.

Comment 3

[André Bargull [:anba]]

Before bug 1346211 we computed the time zone comment from the C-function strftime. That meant in practice:
1. On Windows a long, descriptive, locale-dependent time zone name was used, but only if that name is ASCII-only. If non-ASCII characters are present, no time zone comment string was used at all. (Non-ASCII content was only rejected, because the underlying |char*| string is in some locale-dependent encoding, which makes it difficult to translate it back to a Unicode encoding.)
2. On non-Windows platforms an IANA (or IANA-like) time zone name was used. IANA time zone names are always ASCII, so there were never any encoding issues which may result in returning an empty string for the time zone comment on non-Windows platforms.

Because localised time zone names were already used on Windows, which represents the majority of our user base, and additionally for Chrome-parity (*), I've decided in bug 1346211 to use the localised time zone name for the time zone comment. 

(*) This means the site is also non-functional on Chrome.


We now have the following options:
1. Make no changes and insist the web site code needs to be updated. (A web site update is needed anyway if the web site wants to support Chrome.)
2. Use a localised name, but reject non-ASCII content.
3. Use a localised name, but reject non-ASCII content on Windows. On non-Windows use IANA names. (Basically try to emulate the behaviour before bug 1346211.)
4. Use IANA names on all platforms.
5. Use English names on all platforms.
6. Some combination of the options from above.
7. Always use an empty string for the time zone comment.

Notes:
- Some IANA names are ambiguous. For example PST can mean either "Pacific Standard Time" or "Philippine Standard Time".
- Not all time zones have a short IANA abbreviation, for example Asia/Vladivostok has only "+10". (In the last years IANA removed more and more 'invented' time zone names, so many time zones now only have "+/- hour(minute)" names.)
- Some users like to see localised time zone names (bug 1156728).

Comment 4

[Kohei Yoshino]

The broken site is a Japanese business, so I assume they’ve got JST in most cases.

Comment 5

[André Bargull [:anba]]

(In reply to Kohei Yoshino [:kohei] (Bugzilla UX) (FxSiteCompat) from comment #4)
> The broken site is a Japanese business, so I assume they’ve got JST in most
> cases.

But only on non-Windows systems, on Windows I assume there was no time zone name at all before Firefox 63.

Comment 6

[Kohei Yoshino]

Okay. By the way, the timezone in the `If-Modified-Since` HTTP header must be GMT, apparently. Then the site’s implementation is wrong, and they can solve the issue simply by replacing `toString()` with `toGMTString()`.

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/If-Modified-Since

Comment 7

[Kohei Yoshino]

The original code is in the Teeda JSF framework, which is no longer maintained as of September 2016.

https://github.com/seasarorg/teeda/blob/master/teeda/teeda-ajax/src/main/resources/org/seasar/teeda/ajax/js/ajax.js#L151

Comment 8

[Tom Ritter [:tjr]]

Actually, it turns out that this doesn't bypass RFP; the language prompt you receive (and if you accept we set privacy.spoof_english) causes this to provide a spoofed value.

Comment 9

[Kohei Yoshino]

kabu.com has fixed the issue themselves by using `toUTCString()`. However, the problematic code is part of a framework as mentioned in Comment 7 so there might be other broken sites. Personally I’d expect `toString()` to return only English/ASCII like before. If we want localized strings, `toLocaleString()` can be used instead.

Comment 10

[Pascal Chevrel:pascalc]

Setting as fix-optional in case there is a trivial patch fixing the issue that we could include in a dot release. If we don't get one in the next 2 weeks, it will be wontfix for 64.

Comment 11

[Julien Cristau [:jcristau]]

Should we move this to tech evangelism?

Comment 12

[Kohei Yoshino]

The broken site has fixed the issue themselves so this is not a tech evangelism thing. I still *think* Firefox should restore the previous behaviour.

Comment 13

[Jason Orendorff [:jorendorff]]

We often revert in cases like this, but it's a judgment call. In this case, the change we made was for parity with Chrome, Chrome hasn't reverted, and the change revealed a bug in the site which was then fixed! So I think this change is a good one. Let's keep it.