374040 - nsIScriptableDateFormat does not work as expected with dateFormatLong

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[arno renevier]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.2) Gecko/20070208 Iceweasel/2.0.0.2 (Debian-2.0.0.2+dfsg-3)
Build Identifier: 

nsIScriptableDateFormat.FormatDateTime("", nsIScriptableDateFormat.dateFormatLong, nsIScriptableDateFormat.timeFormatSeconds, ...)
outputs something like:
03/15/07 12:02:47

where we may expect something like :
Thu Mar 15 12:02:47 2007


nsIScriptableDateFormat separates date and time processing and then calls strftime:
http://lxr.mozilla.org/seamonkey/source/intl/locale/src/unix/nsDateTimeFormatUnix.cpp#181

if strftime is given format parameter "%c", it outputs a nice representation of date and time.
Unfortunately, strftime has no parameter to simply output a nice representation of date. So, "%x" parameter is given.
But may be mozilla could check if both dateFormatLong and timeFormatSeconds are set, and, in that case, not separate date and time processing to call strftime with "%c"

Reproducible: Always

Steps to Reproduce:
var date = new Date();
var scriptableDateServ = Components.classes["@mozilla.org/intl/scriptabledateformat;1"].createInstance(Components.interfaces.nsIScriptableDateFormat);

var dateStr = scriptableDateServ.FormatDateTime("", scriptableDateServ.dateFormatLong, scriptableDateServ.timeFormatSeconds, date.getFullYear(), date.getMonth()+1, date.getDate(), date.getHours(), date.getMinutes(), date.getSeconds());

print (dateStr);




testcase:

var date = new Date();
var scriptableDateServ = Components.classes["@mozilla.org/intl/scriptabledateformat;1"].createInstance(Components.interfaces.nsIScriptableDateFormat);

var dateStr = scriptableDateServ.FormatDateTime("", scriptableDateServ.dateFormatLong, scriptableDateServ.timeFormatSeconds, date.getFullYear(), date.getMonth()+1, date.getDate(), date.getHours(), date.getMinutes(), date.getSeconds());

print (dateStr);

Comment 1

[Stephen Donner [:stephend] Not actively reading bugmail]

Confirming; if we want to fix 409317 completely (to be consistent among platforms), we'll have to fix this.

Comment 2

[arno renevier]

Attachment: v1

Comment 3

[Simon Montagu :smontagu]

Does format "%c" automatically honour the locale preferences that we do manually in the kTimeFormatSeconds case below? If so, r=me.

(Minor grumble: a version of the patch with diff -w would have been easier to process)

Comment 4

[arno renevier]

yes, it looks like '%c' honour locale preference:
 LC_TIME=fr_FR.UTF-8 date +'%c'
ven 25 jan 2008 18:02:24 CET
 LC_TIME=en_IN date +'%c'
Friday 25 January 2008 06:02:53 PM CET

also, could kTimeFormatSeconds be computed simplier with just 
PL_strncpy(fmtT, "%X", NSDATETIME_FORMAT_BUFFER_LEN); 
?
(but may be that deserves opening a new bug).

Comment 5

[Simon Montagu :smontagu]

Comment on attachment 297048 [details] [diff] [review]
v1

(In reply to comment #4)
> also, could kTimeFormatSeconds be computed simplier with just 
> PL_strncpy(fmtT, "%X", NSDATETIME_FORMAT_BUFFER_LEN); 
> ?

Yes, I would say so, and you might as well do it while you're here.

Comment 6

[arno renevier]

Attachment: v2

carrying over r+

Comment 7

[arno renevier]

Attachment: v2 with diff -w for easier reading

Comment 8

[Mike Schroepfer]

Date/time parsing can be very fragile - do we have any unit tests for this?

Comment 9

[arno renevier]

Attachment: xpcshell test

Comment 10

[Mike Schroepfer]

Comment on attachment 299551 [details] [diff] [review]
v2

Thnx for the test - approved to checkin both.

Comment 11

[arno renevier]

does that bug needs super review ?

Comment 12

[Simon Montagu :smontagu]

No, you're good to check in.

Comment 13

[Reed Loden [:reed]]

Checking in intl/locale/src/unix/nsDateTimeFormatUnix.cpp;
/cvsroot/mozilla/intl/locale/src/unix/nsDateTimeFormatUnix.cpp,v  <--  nsDateTimeFormatUnix.cpp
new revision: 1.61; previous revision: 1.60
done

Comment 14

[Reed Loden [:reed]]

RCS file: /cvsroot/mozilla/intl/locale/tests/unit/test_bug374040.js,v
done
Checking in intl/locale/tests/unit/test_bug374040.js;
/cvsroot/mozilla/intl/locale/tests/unit/test_bug374040.js,v  <--  test_bug374040.js
initial revision: 1.1
done

Comment 15

[Robert Kaiser]

This test fails on the SeaMonkey tinderbox with:
[Exception... "Component returned failure code: 0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]"  nsresult: "0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE)"  location: "JS frame :: ../../../_tests/xpcshell-simple/test_intl_locale/unit/test_bug374040.js :: run_test :: line 35"  data: no]

http://mxr.mozilla.org/seamonkey/source/modules/libpr0n/test/unit/test_imgtools.js#135 says that nsIXULRuntime.OS might have problems in xpcshell, can that be related?

Comment 16

[Jeff Walden [:Waldo]]

I'm also seeing this test fail on OS X, using the usual method for running the tests -- it's complaining that Cc[xulruntime] is undefined.  Curious -- would tinderboxen using objdir affect this?  As it is, I can't run xpcshell tests except in pieces.

Comment 17

[Robert Kaiser]

Why even test for OS here? Can't we just test for a feature, or can't we run the test for all OSes?

Comment 18

[arno renevier]

I test os because my patch is unix specific, and in unix nsIScriptableDateFormat with dateFormatLong,timeFormatNone is equivalent to date.toLocaleDateString: it uses strftime(..., "%X"); nsIScriptableDateFormat with dateFormatLong,timeFormatLong is equivalent to date.toLocaleString: it uses strftime(..., "%c"); I don't known if it's the same for other OSes. If so, OS detection is not needed.

Also, I can run xpcshell tests either with  objdir or not. Is the problem (Cc[xulruntime] undefined) os specific ?

Comment 19

[Robert Kaiser]

I thought we are supposed to be cross-platform and have the same results for common functions all all of those...

Anyways, 1) the problem does seem to be different on different machines, not necessarily per OS, maybe it's even different depending on what app the test runs in, as Firefox tinderboxen don't fail so far, and 2) checking for a handful of unix variants, not knowing if we might add others some time, which might again need a change there sounds suboptimal anyways, is there no better variant to run a test on unix only?

Comment 20

[arno renevier]

Attachment: xpcshell test in unix directory

date formatting in intl/locale is handled differently by each platform. May be that could be changed (for example, js/src/prmjtime.c needs less platform specific code).

About platform specific, what would happen if tests were in intl/locale/src/unix/tests ? would tinderbox tests run them or not ?

Comment 21

[Robert Kaiser]

From what people tell me, this sounds exactly the right way to do it, thanks!

Reed, could you please check this in?

Comment 22

[Reed Loden [:reed]]

Checking in intl/locale/src/unix/Makefile.in;
/cvsroot/mozilla/intl/locale/src/unix/Makefile.in,v  <--  Makefile.in
new revision: 1.26; previous revision: 1.25
done
RCS file: /cvsroot/mozilla/intl/locale/src/unix/tests/Makefile.in,v
done
Checking in intl/locale/src/unix/tests/Makefile.in;
/cvsroot/mozilla/intl/locale/src/unix/tests/Makefile.in,v  <--  Makefile.in
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/intl/locale/src/unix/tests/unit/test_bug374040.js,v
done
Checking in intl/locale/src/unix/tests/unit/test_bug374040.js;
/cvsroot/mozilla/intl/locale/src/unix/tests/unit/test_bug374040.js,v  <--  test_bug374040.js
initial revision: 1.1
done
Removing intl/locale/tests/unit/test_bug374040.js;
/cvsroot/mozilla/intl/locale/tests/unit/test_bug374040.js,v  <--  test_bug374040.js
new revision: delete; previous revision: 1.1
done

Comment 23

[Shawn Wilsher :sdwilsh]

Did this cause bug 415361?

Comment 24

[Dave Townsend [:mossop]]

I have had to disable the test for this which is failing due to bug 421790