307895 - Date.toLocalFormat("%x") with format yyyy/MM/dd produces yyyy/MM/yyyy

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[gekacheka]

Date toLocalFormat("%x") with format yyyy/MM/dd produces yyyy/MM/yyyy

1. set o.s. date format to yyyy/MM/dd (w2k: control panel | regional options)
2. (re)start app (ff, tb, etc. since 2005-05-05 [bug 291494 checkin])
3. Tools | JavaScript console
4. new Date().toLocaleFormat("%x")

Actual results:
  2005/09/2005
Expected results:
  2005/09/10

This is serious in that yyyy/MM/dd is the default short date format for some
locales such as en-ZA (south africa).

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

this seems to have been done sorta intentionally?
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/js/src/jsdate.c&mark=1770-1775#1769

Comment 2

[gekacheka]

Attachment: don't hack digits at end if starts with four digits (untested)

(patch -l -p 2 -i file.patch)

possible approach: add check to make sure date does not start with four digit
year.  Also adds length check so doesn't write outside buffer if length is 1,
and broadens hack so it applies even if '/' is not used as digit separator.

Comment 3

[Brendan Eich [:brendan]]

(In reply to comment #1)
> this seems to have been done sorta intentionally?
>
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/js/src/jsdate.c&mark=1770-1775#1769

See bug 61183.  Patch comments anon.

/be

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 195558 [details] [diff] [review]
don't hack digits at end if starts with four digits (untested)

>diff -rubtp -x '*[#~]' mozilla/js/src/jsdate.c ../cvs20050910a/mozilla/js/src/jsdate.c
>--- mozilla/js/src/jsdate.c	2005-09-06 02:50:45.000000000 -0400
>+++ ../cvs20050910a/mozilla/js/src/jsdate.c	2005-09-10 13:45:10.219032000 -0400
>@@ -1768,8 +1768,13 @@ date_toLocaleHelper(JSContext *cx, JSObj
>             return date_format(cx, *date, FORMATSPEC_FULL, rval);
> 
>         /* Hacked check against undesired 2-digit year 00/00/00 form. */
>-        if (buf[result_len - 3] == '/' &&
>-            isdigit(buf[result_len - 2]) && isdigit(buf[result_len - 1])) {
>+        if (result_len >= 4 &&

Good to check result_len here, but should we insist that it's >= 6?

>+            /* hack 3/11/22 or 11.03.22 or 11Mar22 */
>+            !isdigit(buf[result_len - 3]) &&

Why change this from the exact match against '/'?

>+            isdigit(buf[result_len - 2]) && isdigit(buf[result_len - 1]) &&
>+            /* but not 2022/3/11 */
>+            !(isdigit(buf[0]) && isdigit(buf[1]) &&
>+              isdigit(buf[2]) && isdigit(buf[3]))) {
>             JS_snprintf(buf + (result_len - 2), (sizeof buf) - (result_len - 2),
>                         "%d", js_DateGetYear(cx, obj));
>         }

Looks good otherwise, minusing _pro forma_ but looking forward to a patch
addressing the above two questions.

/be

Comment 5

[gekacheka]

Attachment: v2: don't hack digits at end if not %x or starts with four digits (untested)

(patch -l -p 2 -i file.patch)

1. changed length test to 6 as requested
2. The hack is meant to apply to toLocaleDateString (bug 61183) which supplies
the %x format so the user's operating system locale settings will be used.  The
 settings may use a date separator other than '/', (for example, Germany uses
'.'), so should check for nondigit rather than only for '/'.
3. The hack should apply only if format is %x, which means use the
  operating system locale settings.  Otherwise get strange behavior when 
  programs specify an explicit format, as follows:
    new Date().toLocaleFormat("%d/%m/%y") --> 10/09/2005
    new Date().toLocaleFormat("%y/%m/%d") --> 05/09/10
  So added check that format is "%x".  (Do not need to check for %#x
  for windows because that specifically produces a four digit year.)

Comment 6

[Brendan Eich [:brendan]]

Comment on attachment 195579 [details] [diff] [review]
v2: don't hack digits at end if not %x or starts with four digits (untested)

r=me, thanks -- this can go into the 1.9a trunk right away.  Do you need
someone to check it in for you?

/be

Comment 7

[gekacheka]

(In reply to comment #6)
> (From update of attachment 195579 [details] [diff] [review] [edit])
> r=me, thanks -- this can go into the 1.9a trunk right away.  Do you need
> someone to check it in for you?
> 
> /be

Please do, thank you.

Comment 8

[Stefan Sitter [:ssitter]]

Problem still exist in current trunk build. Seems that the patch is not yet 
checked in.

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

I just checked this patch into the trunk. Sorry for not doing this sooner.

Comment 10

[Bob Clary [:bc] (inactive)]

man strftime

       `%x'
            A string representing the complete date, equivalent to "%m/%d/%y".
            [tm_mon, tm_mday, tm_year]

       `%m'
            The month number, formatted with two digits (from ``01'' to
            ``12'').  [tm_mon]

       `%d'
            The day of the month, formatted with two digits (from ``01'' to
            ``31''). [tm_mday]

       `%y'
            The last two digits of the year (from ``00'' to ``99''). [tm_year]

winxp branch passes js1_5/Date/toLocaleFormat.js but trunk fails after this 
patch with 

Date.toLocaleFormat("%m/%d/%y) == Date.toLocaleFormat("%x")
Expected value '06/04/05', Actual value '06/04/2005'

Comment 11

[gekacheka]

(In reply to comment #10)
> man strftime
> 
>        `%x'
>             A string representing the complete date, equivalent to "%m/%d/%y".

Maybe your strftime manual is nonstandard or old?  The standard version produces the date formatted according to the current locale settings.

Compare to 

http://www.opengroup.org/onlinepubs/007908799/xsh/strftime.html

  %x
      is replaced by the locale's appropriate date representation.


http://man.linuxquestions.org/index.php?query=strftime&section=3&type=2&print=1

  %x     The preferred date representation for the current locale without
	 the time.


http://developer.apple.com/documentation/Darwin/Reference/ManPages/man3/strftime.3.html

  %x    is replaced by national representation of the date.
  ...
  The strftime() function conforms to ISO/IEC 9899:1990 ...


http://msdn.microsoft.com/library/default.asp?url=/library/en-us/vclib/html/_crt_strftime.2c_.wcsftime.asp

  %x
      Date representation for current locale


I guess the test at http://lxr.mozilla.org/mozilla/source/js/tests/js1_5/Date/toLocaleFormat.js#219
needs to allow a customizable date format.  Maybe instead it can check that the result string is a different string for each day of month, for each month of year, and for different years.

Comment 12

[Stefan Sitter [:ssitter]]

What I think:

The "Hacked check against undesired 2-digit year 00/00/00 form." has been in jsdate.c since 2000-11-29.

Until now date_toLocaleHelper() always replaced the last two digits with a 4-digit year (with today as example):
  new Date().toLocaleFormat("%m/%d/%y) --> 10/24/2005
  new Date().toLocaleFormat("%x")      --> 10/24/2005

Thus the test passed until now. (At least if your local date format setting is MM/dd/yy. For me .toLocaleFormat("%x") returns 24.10.2005 as expected.)

But after checkin for this bug date_toLocaleHelper() replaces just in case of format "%x":
  new Date().toLocaleFormat("%m/%d/%y) --> 10/24/05
  new Date().toLocaleFormat("%x")      --> 10/24/2005

Thus the test fails now.

Comment 13

[Simon Paquet [:sipaq]]

*** Bug 312860 has been marked as a duplicate of this bug. ***

Comment 14

[YAEGASHI Takeshi]

Hi there,

This bug seems still outstanding in the latest Firefox/1.5 build.  I see new Date().toLocaleFormat("%x") returns 2006/01/2006 on Japanese Windows 2000, which causes Calendar's crash as reported in Bug 312860.

The build identifier is: Mozilla/5.0 (Windows; U; Windows NT 5.0; ja; rv:1.8) Gecko/20051111 Firefox/1

Comment 15

[YAEGASHI Takeshi]

(In reply to comment #14)
> The build identifier is: Mozilla/5.0 (Windows; U; Windows NT 5.0; ja; rv:1.8)
> Gecko/20051111 Firefox/1

Sorry, some characters were truncated.  The correct one is:
Mozilla/5.0 (Windows; U; Windows NT 5.0; ja; rv:1.8) Gecko/20051111 Firefox/1.5

Comment 16

[Bob Clary [:bc] (inactive)]

(In reply to comment #15)

this was fixed on the trunk not the 1.8 branch.

Comment 17

[YAEGASHI Takeshi]

(In reply to comment #16)
> this was fixed on the trunk not the 1.8 branch.

Yes, I meant that.  Actually, is there any chance for me to have this bug fixed in the forthcoming 1.8.x release? 

I'm not familiar with the project, so if it's inappropriate to post such a wish on the bugzilla, please guide me to the right place.  Thanks.

Comment 18

[Bob Clary [:bc] (inactive)]

You can nominate them by setting the blocking flags for the bug. I will do that for 1.8.0.2 which will be in a couple of months and 1.8.1 which will be Firefox 2. It is too late for 1.8.0.1.

Comment 19

[Daniel Veditz [:dveditz]]

Doesn't fit the scope of a security/stability release like 1.8.0.2

Comment 20

[Stefan Sitter [:ssitter]]

Is this fixed on trunk only? I think it should be fixed on MOZILLA_1_8_BRANCH for Firefox 2.0 and Thunderbird 2.0 too.

Comment 21

[Mike Beltzner [:beltzner, not reading bugmail]]

Can we get this landed on the branch please?

Comment 22

[Mike Beltzner [:beltzner, not reading bugmail]]

Is this part of the JS 1.7 landing? If not, can we please have this checked into the 1.8.1 branch ASAP?

Comment 23

[Brendan Eich [:brendan]]

Fixed with JS1.7 landing.

/be

Comment 24

[Bob Clary [:bc] (inactive)]

verified fixed 1.8.1, 1.9 winxp.