429492 - Cleanup ISO8601DateUtils.jsm

Status: RESOLVED WONTFIX

(Core :: XPConnect, defect)

Description

[Takeshi Kurosawa :takenspc]

Attachment: Patch rv. 1.0

|ISO8601DateUtils.create| returns |YYYYMMDDThh:mm:ssZ| now. Below is log of xpcshell

js> Components.utils.import("resource://gre/modules/ISO8601DateUtils.jsm");
[object BackstagePass]
js> ISO8601DateUtils.create(new Date);
20080417T12:15:55Z

But |YYYY-MM-DDThh:mm:ssZ| is more common. And its comment says

> // YYYY-MM-DDThh:mm:ssZ

We should use |YYYY-MM-DDThh:mm:ssZ|

Comment 1

[YUKI "Piro" Hiroshi]

I think we MUST use YYYY-MM-DDThh:mm:ssZ style, not YYYYMMDDThh:mm:ssZ.
The ISO 8601:2004 allows you to remove hyphens, but W3C-DTF doesn't.
W3C-DTF says:

> Different standards may need different levels of granularity in the date
> and time, so this profile defines six levels. Standards that reference this
> profile should specify one or more of these granularities. If a given standard
> allows more than one granularity, it should specify the meaning of the dates
> and times with reduced precision, for example, the result of comparing two
> dates with different precisions.
>
> The formats are as follows. Exactly the components shown here must be present,
> with exactly this punctuation. Note that the "T" appears literally in the
> string, to indicate the beginning of the time element, as specified in ISO
> 8601.
( http://www.w3.org/TR/NOTE-datetime )

There are only 6 patterns, and all of them include hyphens. There is no sample
without hyphens.

>   Year:
>      YYYY (eg 1997)
>   Year and month:
>      YYYY-MM (eg 1997-07)
>   Complete date:
>      YYYY-MM-DD (eg 1997-07-16)
>   Complete date plus hours and minutes:
>      YYYY-MM-DDThh:mmTZD (eg 1997-07-16T19:20+01:00)
>   Complete date plus hours, minutes and seconds:
>      YYYY-MM-DDThh:mm:ssTZD (eg 1997-07-16T19:20:30+01:00)
>   Complete date plus hours, minutes, seconds and a decimal fraction of a
>second
>      YYYY-MM-DDThh:mm:ss.sTZD (eg 1997-07-16T19:20:30.45+01:00)
( http://www.w3.org/TR/NOTE-datetime )

"ISO8601DateUtils.jsm" says:

> var ISO8601DateUtils = {
> 
>   /**
>   * XXX Thunderbird's W3C-DTF function
>   *
>   * Converts a W3C-DTF (subset of ISO 8601) date string to a Javascript
>   * date object. W3C-DTF is described in this note:
>   * http://www.w3.org/TR/NOTE-datetime IETF is obtained via the Date
>   * object's toUTCString() method.  The object's toString() method is
>   * insufficient because it spells out timezones on Win32
>   * (f.e. "Pacific Standard Time" instead of "PST"), which Mail doesn't
>   * grok.  For info, see
>   * http://lxr.mozilla.org/mozilla/source/js/src/jsdate.c#1526.
>   */

But current implementation is invalid at W3C-DTF.

Comment 2

[drry+mozilla]

Attachment: Yet another patch rv.1.

2008
+0020
+002008
2008-04
2008111
2008-111
20080420
2008W167
2008-W16-7
2008-04-20
2008-04-20T00
200804200000,0
2008-04-20T00Z
2008-04-20T00,0Z
2008-04-20T00+00
2008-04-20T00+00:00
2008-04-20T00:00,0Z
2008-04-20T00:00:00Z
2008-04-20T00:00:00,0Z
2008-04-20T00:00:00.000Z
2008-04-20T00:00:00.000+00:00
2008-04-20T24:00:00Z
2006-01-01T08:59:60+09:00

Comment 3

[drry+mozilla]

Attachment: Yet another patch rv.2.

added some explanations and me as contributors.

Comment 4

[Takeshi Kurosawa :takenspc]

drry-san's patch cleans up the entire of ISO8601DateUtils.jsm.
# resolves the issue of |create| too.

So, I changed the summary and reset the assignee.

drry-san, could you accept this bug?

Comment 5

[Takeshi Kurosawa :takenspc]

Comment on attachment 316221 [details] [diff] [review]
Patch rv. 1.0

obsolete my patch.

Comment 6

[Takeshi Kurosawa :takenspc]

Oops! There is no ISO8601.jsm. The right filename is ISO8601DateUtils.jsm  :-(

Comment 7

[drry+mozilla]

Attachment: Yet another patch rv.3.

(In reply to comment #4)
> drry-san, could you accept this bug?
No problem, sir.

Comment 8

[drry+mozilla]

Attachment: Yet another patch rv.4.

Comment 9

[drry+mozilla]

Attachment: Yet another patch rv.5.

Comment 10

[drry+mozilla]

Attachment: Yet another patch rv.6.

Comment 11

[drry+mozilla]

Attachment: Yet another patch rv.7.

Comment 12

[drry+mozilla]

Attachment: Yet another patch rv.8.

let date = new Date();
let string = ISO8601DateUtils.create(date);
print("1: " + string);
print("2: " + ISO8601DateUtils.create(date, ISO8601DateUtils.EXTENDED_FORMAT));
print("3: " + ISO8601DateUtils.create(date, ISO8601DateUtils.BASIC_FORMAT));
print("4: " + ISO8601DateUtils.create(date, ISO8601DateUtils.W3CDT_FORMAT | ISO8601DateUtils.AS_LOCAL_TIME));
print("5: " + ISO8601DateUtils.create(date, ISO8601DateUtils.NO_TIMEZONE));
date.setMilliseconds(0);
print("6: " + ISO8601DateUtils.create(date));
print("7: " + date);
print("8: " + ISO8601DateUtils.parse(string));
date = ISO8601DateUtils.parse("2008-115");
print("9: " + date);
print("10: " + date.hasTime);
print("11: " + ISO8601DateUtils.create(date));
print("12: " + ISO8601DateUtils.parse("2008W174"));

Results:
1: 2008-04-23T22:07:31.827Z
2: 2008-04-23T22:07:31,827Z
3: 20080423T220731,827Z
4: 2008-04-24T07:07:31.827+09:00
5: 2008-04-23T22:07:31,827
6: 2008-04-23T22:07:31Z
7: Thu Apr 24 2008 07:07:31 GMT+0900
8: Thu Apr 24 2008 07:07:31 GMT+0900
9: Thu Apr 24 2008 00:00:00 GMT+0900
10: false
11: 2008-04-23
12: Thu Apr 24 2008 00:00:00 GMT+0900

Comment 13

[drry+mozilla]

Attachment: Yet another patch rv.9.

added two default options |WITH_TIME| and |NO_MILLISECONDS|.

Comment 14

[drry+mozilla]

Attachment: Yet another patch rv.10.

added the leap year and non "minus zero" checking.

Comment 15

[drry+mozilla]

Attachment: Yet another patch rv.11.

Trivial fixes.

Comment 16

[Erwan Loisant]

Attachment: Test case for date parsing

The patch actually does not do just cleanup; it changes the behavior of the module. Since we need to make sure we don't break the existing behavior, I think we should separate code cleanup (that doesn't affect the behavior) from bug fixes. And indeed there are bugs to be fixed, Bug 439295 for example. Note that this module is used in the feed parser, so if we mess up we can have problems in the dates displayed in the feed preview.

I prepared a simple test case for date parsing, so we know how the module is supposed to behave on a few cases. It currently fails because of Bug 439295 (that's the last check). Since you guys seems to know about the ISO8601 date standard better than me feel free to complete/change it, but I think we should at least document any change in the behavior.

Some dates are parsed differently with and without the patch (did it became intolerant to the missing "Z" at the end?).

Comment 17

[kitchin]

Should be WONTFIX, due to Bug 543535 - Remove ISO8601DateUtils.jsm

Comment 18

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Indeed.