Bug 160287 - [JSC] DateMath should accept more ISO-8601 timezone designators even if they are not included in ECMA262 to produce expected results in the wild code
Summary: [JSC] DateMath should accept more ISO-8601 timezone designators even if they ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-07-28 02:18 PDT by florian
Modified: 2020-01-23 09:56 PST (History)
14 users (show)

See Also:


Attachments
Patch (14.74 KB, patch)
2020-01-22 00:14 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (15.18 KB, patch)
2020-01-22 00:17 PST, Yusuke Suzuki
ross.kirsling: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description florian 2016-07-28 02:18:51 PDT
If I try to parse a string which contains a date without a colon between hour and minute of timezone it is an invalid date.

Code to reproduce:
```
new Date('2016-07-27T16:42:16+0000');
```

or if you try to parse a date string:
```
Date.parse('2016-07-27T16:42:16+0000');
```
Comment 1 Alexey Proskuryakov 2016-07-30 00:17:42 PDT
Does this work in other browsers?
Comment 2 florian 2016-08-08 03:22:15 PDT
(In reply to comment #1)
> Does this work in other browsers?

Yes, I tested it in Chrome and Firefox. This is the paragraph (on Wikipedia) which describes the time zone designators and where you can find the different formats: https://en.wikipedia.org/wiki/ISO_8601#Time_zone_designators
Comment 3 radio 2019-09-16 04:56:29 PDT
This seems to still be an issue.  

I connected the Safari inspector from my computer to an iPhone running iOS 12.4, then opened a console:

> new Date("2019-11-26T23:59:59-0800")
< Invalid Date
> new Date("2019-11-26T23:59:59-08:00")
< Wed Nov 27 2019 07:59:59 GMT+0000 (WET)

Per the ISO 8601 spec, it seems that a time zone offset of "-0800" should be acceptable.  But mobile Safari is only accepting "-08:00".


https://en.wikipedia.org/wiki/ISO_8601#Times
Comment 4 Jack Koppa 2019-12-19 12:43:33 PST
Can confirm that I see this issue as well. The examples posted will indeed parse correctly in Firefox & Chrome.

Examples of other people coming across the problem:

http://farai.github.io/blog/2015/02/16/javascript-nan-for-new-date-object-in-ie-and-safari/

https://stackoverflow.com/a/49138448/4167438
Comment 5 Yusuke Suzuki 2020-01-21 22:41:47 PST

*** This bug has been marked as a duplicate of bug 89071 ***
Comment 6 Yusuke Suzuki 2020-01-21 22:45:22 PST
No, the bug 89071 is related to no-timezone format.
Comment 7 Yusuke Suzuki 2020-01-21 22:47:25 PST
(In reply to radio from comment #3)
> https://en.wikipedia.org/wiki/ISO_8601#Times

https://tc39.es/ecma262/#sec-date-time-string-format

It seems that ":" in timezone is necessary according to the spec.

> Z	is the UTC offset representation specified as "Z" (for UTC with no offset) or an offset of either "+" or "-" followed by a time expression HH:mm (indicating local time ahead of or behind UTC, respectively)

And looks like ECMA262's format is simplified version of ISO-8601.

> ECMAScript defines a string interchange format for date-times based upon a simplification of the ISO 8601 calendar date extended format. The format is as follows: YYYY-MM-DDTHH:mm:ss.sssZ

@Ross, do you know why are ECMA262 and ISO-8601 different? Is there any reasons?
Comment 8 Yusuke Suzuki 2020-01-21 23:02:09 PST
Maybe, we could relax our parser to accept more formats than the things specified in ECMA262, especially if it is included in ISO-8601, including +hh and +hhmm.
Comment 9 Yusuke Suzuki 2020-01-21 23:03:12 PST
(In reply to Yusuke Suzuki from comment #8)
> Maybe, we could relax our parser to accept more formats than the things
> specified in ECMA262, especially if it is included in ISO-8601, including
> +hh and +hhmm.

And maybe, we should file a bug to ECMA262 :)
Comment 10 Radar WebKit Bug Importer 2020-01-21 23:04:53 PST
<rdar://problem/58787924>
Comment 11 Yusuke Suzuki 2020-01-22 00:14:13 PST
Created attachment 388400 [details]
Patch
Comment 12 Yusuke Suzuki 2020-01-22 00:17:14 PST
Created attachment 388401 [details]
Patch
Comment 13 Ross Kirsling 2020-01-22 11:18:42 PST
Comment on attachment 388401 [details]
Patch

r=me, nice.

Do you want to submit the ECMA-262 issue or would you rather I do so?
Comment 14 Ross Kirsling 2020-01-22 11:46:07 PST
Comment on attachment 388401 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=388401&action=review

> JSTests/stress/relaxed-timezone-designators.js:96
> +shouldBe(parsedDate(`1970-01-01T00:00:00.000+24:59`), -((24 * 60 + 59) * 60 * 1000));
> +shouldBe(parsedDate(`1970-01-01T00:00:00.000-24:59`), +((24 * 60 + 59) * 60 * 1000));
> +shouldBe(parsedDate(`1970-01-01T00:00:00.000+2459`), -((24 * 60 + 59) * 60 * 1000));
> +shouldBe(parsedDate(`1970-01-01T00:00:00.000-2459`), +((24 * 60 + 59) * 60 * 1000));

Er, wait a sec. I guess this is a pre-existing thing but should we really be supporting 24:xx in general?

For instance, this article (https://www.cl.cam.ac.uk/~mgk25/iso-time.html) says:
"If the hour value is 24, then the minute and second values must be zero."

It appears that V8 and SM do not support 24 as an hour offset (even for 24:00), while ChakraCore and XS do (even for 24:59).

This W3C note also suggests that they don't expect 24 to be a supported value: https://www.w3.org/TR/NOTE-datetime
Comment 15 Yusuke Suzuki 2020-01-22 12:54:12 PST
(In reply to Ross Kirsling from comment #14)
> Comment on attachment 388401 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=388401&action=review
> 
> > JSTests/stress/relaxed-timezone-designators.js:96
> > +shouldBe(parsedDate(`1970-01-01T00:00:00.000+24:59`), -((24 * 60 + 59) * 60 * 1000));
> > +shouldBe(parsedDate(`1970-01-01T00:00:00.000-24:59`), +((24 * 60 + 59) * 60 * 1000));
> > +shouldBe(parsedDate(`1970-01-01T00:00:00.000+2459`), -((24 * 60 + 59) * 60 * 1000));
> > +shouldBe(parsedDate(`1970-01-01T00:00:00.000-2459`), +((24 * 60 + 59) * 60 * 1000));
> 
> Er, wait a sec. I guess this is a pre-existing thing but should we really be
> supporting 24:xx in general?
> 
> For instance, this article (https://www.cl.cam.ac.uk/~mgk25/iso-time.html)
> says:
> "If the hour value is 24, then the minute and second values must be zero."
> 
> It appears that V8 and SM do not support 24 as an hour offset (even for
> 24:00), while ChakraCore and XS do (even for 24:59).
> 
> This W3C note also suggests that they don't expect 24 to be a supported
> value: https://www.w3.org/TR/NOTE-datetime

I think it is OK (while it is a bit fun), Date.parse spec has requirement that particular format should be parsed.
But it allows engines to accept more formats if engines want. So, for now, I'm just leaving 24 handling :)
Comment 16 Yusuke Suzuki 2020-01-22 12:54:32 PST
(In reply to Ross Kirsling from comment #13)
> Comment on attachment 388401 [details]
> Patch
> 
> r=me, nice.
> 
> Do you want to submit the ECMA-262 issue or would you rather I do so?

Can you submit it?
Comment 17 Yusuke Suzuki 2020-01-22 12:55:34 PST
Committed r254939: <https://trac.webkit.org/changeset/254939>
Comment 18 Ross Kirsling 2020-01-23 09:56:12 PST
(In reply to Yusuke Suzuki from comment #16)
> (In reply to Ross Kirsling from comment #13)
> > Comment on attachment 388401 [details]
> > Patch
> > 
> > r=me, nice.
> > 
> > Do you want to submit the ECMA-262 issue or would you rather I do so?
> 
> Can you submit it?

https://github.com/tc39/ecma262/issues/1853