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.

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.

carrying over r+

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

Comment on attachment 299551 [details] [diff] [review] v2 Thnx for the test - approved to checkin both.

does that bug needs super review ?

No, you're good to check in.

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

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

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 ?

From what people tell me, this sounds exactly the right way to do it, thanks! Reed, could you please check this in?

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

Did this cause bug 415361?

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