Closed Bug 374040 Opened 15 years ago Closed 14 years ago

nsIScriptableDateFormat does not work as expected with dateFormatLong

Categories

(Core :: Internationalization, defect)

x86
Linux
defect
Not set
minor

Tracking

()

RESOLVED FIXED
mozilla1.9beta3

People

(Reporter: arno, Assigned: arno)

References

Details

Attachments

(4 files, 1 obsolete file)

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);
Confirming; if we want to fix 409317 completely (to be consistent among platforms), we'll have to fix this.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attached patch v1 (obsolete) — Splinter Review
Assignee: smontagu → arno.@no-log.org
Status: NEW → ASSIGNED
Attachment #297048 - Flags: review?(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)
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 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.
Attachment #297048 - Flags: review?(smontagu) → review+
Attached patch v2Splinter Review
carrying over r+
Attachment #297048 - Attachment is obsolete: true
Attachment #299551 - Flags: review+
Attachment #299551 - Flags: approval1.9?
Date/time parsing can be very fragile - do we have any unit tests for this?
Flags: in-testsuite?
Attached patch xpcshell testSplinter Review
Comment on attachment 299551 [details] [diff] [review]
v2

Thnx for the test - approved to checkin both.
Attachment #299551 - Flags: approval1.9? → approval1.9+
Keywords: helpwanted
Keywords: helpwanted
does that bug needs super review ?
No, you're good to check in.
Keywords: checkin-needed
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
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9beta3
Version: unspecified → Trunk
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
Flags: in-testsuite? → in-testsuite+
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?
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.
Why even test for OS here? Can't we just test for a feature, or can't we run the test for all OSes?
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 ?
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?
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 ?
Blocks: 414877
Attachment #300679 - Flags: review+
From what people tell me, this sounds exactly the right way to do it, thanks!

Reed, could you please check this in?
Keywords: checkin-needed
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
Keywords: checkin-needed
Did this cause bug 415361?
I have had to disable the test for this which is failing due to bug 421790
Flags: in-testsuite+
Blocks: 468835
Blocks: 140814
You need to log in before you can comment on or make changes to this bug.