Cleanup ISO8601DateUtils.jsm

RESOLVED WONTFIX

Status

()

Core
XPConnect
RESOLVED WONTFIX
9 years ago
6 years ago

People

(Reporter: Takeshi Kurosawa, Assigned: drry+mozilla)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 11 obsolete attachments)

(Reporter)

Description

9 years ago
Created attachment 316221 [details] [diff] [review]
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|
Attachment #316221 - Flags: review?(sayrer)
(Reporter)

Updated

9 years ago
Status: NEW → ASSIGNED

Comment 1

9 years ago
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.
(Assignee)

Comment 2

9 years ago
Created attachment 316688 [details] [diff] [review]
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
(Assignee)

Updated

9 years ago
Attachment #316688 - Attachment is obsolete: true
(Assignee)

Comment 3

9 years ago
Created attachment 316740 [details] [diff] [review]
Yet another patch rv.2.

added some explanations and me as contributors.
(Reporter)

Comment 4

9 years ago
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?
Status: ASSIGNED → NEW
Summary: ISO8601DateUtils.create should use YYYY-MM-DD instead of YYYYMMDD for date → Cleanup ISO8601.jsm
(Reporter)

Comment 5

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

obsolete my patch.
Attachment #316221 - Attachment is obsolete: true
Attachment #316221 - Flags: review?(sayrer)
(Reporter)

Comment 6

9 years ago
Oops! There is no ISO8601.jsm. The right filename is ISO8601DateUtils.jsm  :-(
Summary: Cleanup ISO8601.jsm → Cleanup ISO8601DateUtils.jsm
(Assignee)

Updated

9 years ago
Attachment #316740 - Attachment is obsolete: true
(Assignee)

Comment 7

9 years ago
Created attachment 316928 [details] [diff] [review]
Yet another patch rv.3.

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

Comment 8

9 years ago
Created attachment 316935 [details] [diff] [review]
Yet another patch rv.4.
Attachment #316928 - Attachment is obsolete: true
(Assignee)

Comment 9

9 years ago
Created attachment 316948 [details] [diff] [review]
Yet another patch rv.5.
Attachment #316935 - Attachment is obsolete: true
(Assignee)

Comment 10

9 years ago
Created attachment 316978 [details] [diff] [review]
Yet another patch rv.6.
Attachment #316948 - Attachment is obsolete: true
(Assignee)

Comment 11

9 years ago
Created attachment 317017 [details] [diff] [review]
Yet another patch rv.7.
Attachment #316978 - Attachment is obsolete: true
(Reporter)

Updated

9 years ago
Assignee: nobody → drry+mozilla
(Assignee)

Updated

9 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 12

9 years ago
Created attachment 317399 [details] [diff] [review]
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
Attachment #317017 - Attachment is obsolete: true
Attachment #317399 - Flags: review?(sayrer)
(Assignee)

Comment 13

9 years ago
Created attachment 317478 [details] [diff] [review]
Yet another patch rv.9.

added two default options |WITH_TIME| and |NO_MILLISECONDS|.
Attachment #317399 - Attachment is obsolete: true
Attachment #317399 - Flags: review?(sayrer)
(Assignee)

Comment 14

9 years ago
Created attachment 317956 [details] [diff] [review]
Yet another patch rv.10.

added the leap year and non "minus zero" checking.
Attachment #317478 - Attachment is obsolete: true
Attachment #317956 - Flags: review?(sayrer)
(Assignee)

Comment 15

9 years ago
Created attachment 319134 [details] [diff] [review]
Yet another patch rv.11.

Trivial fixes.
Attachment #317956 - Attachment is obsolete: true
Attachment #319134 - Flags: review?(sayrer)
Attachment #317956 - Flags: review?(sayrer)
(Assignee)

Updated

9 years ago
Blocks: 398177

Comment 16

9 years ago
Created attachment 325278 [details]
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?).

Updated

9 years ago
Blocks: 393966

Comment 17

6 years ago
Should be WONTFIX, due to Bug 543535 - Remove ISO8601DateUtils.jsm
Indeed.
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Component: Web Services → XPConnect
QA Contact: web-services → xpconnect
Resolution: --- → WONTFIX

Updated

6 years ago
Attachment #319134 - Flags: review?(sayrer)
You need to log in before you can comment on or make changes to this bug.