Last Comment Bug 603159 - implement exslt-date:date-time()
: implement exslt-date:date-time()
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: XSLT (show other bugs)
: unspecified
: All All
: -- enhancement with 1 vote (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
: Andrew Overholt [:overholt]
Mentors:
http://greenbytes.de/tech/webdav/xslt...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-10-10 01:41 PDT by Julian Reschke
Modified: 2011-08-09 01:12 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
implementation of exslt:date-time() (6.06 KB, patch)
2010-10-10 01:43 PDT, Julian Reschke
no flags Details | Diff | Splinter Review
implementation of exslt:date-time(), plus a minimal test case (8.57 KB, patch)
2011-04-10 06:51 PDT, Julian Reschke
jonas: review-
Details | Diff | Splinter Review
implementation of exslt:date-time(), plus a minimal test case (8.82 KB, patch)
2011-04-19 08:06 PDT, Julian Reschke
jonas: review+
Details | Diff | Splinter Review
implementation of exslt:date-time(), plus a minimal test case (6.59 KB, patch)
2011-04-26 05:51 PDT, Julian Reschke
jonas: review+
Details | Diff | Splinter Review

Description Julian Reschke 2010-10-10 01:41:43 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.10) Gecko/20100914 Firefox/3.6.10 (.NET CLR 3.5.30729)
Build Identifier: 3.6.10

The EXSLT date-time module contains many functions (http://www.exslt.org/date/index.html). However, implementing just date-time() out of these would be very useful, because it enables XSLTs to get the current system date (something that currently can't done at all).

Reproducible: Always

Steps to Reproduce:
Navigate to http://greenbytes.de/tech/webdav/xslt-test.xml
Actual Results:  
"Date-Time Support

exslt:date-time(): No"

Expected Results:  
"Date-Time Support

exslt:date-time(): Yes, and returns: 2010-10-10T09:40:29.095+01:00"

(with the current date)
Comment 1 Julian Reschke 2010-10-10 01:43:19 PDT
Created attachment 482123 [details] [diff] [review]
implementation of exslt:date-time()

1) the string handling probably should be double-checked

2) is there a canonical place for a test case?
Comment 2 C. M. Sperberg-McQueen 2010-11-13 03:02:10 PST
Having date-time() would certainly be useful.  I hope it can happen.
Comment 3 Julian Reschke 2011-04-05 10:11:03 PDT
Any chance to get this into FF5?
Comment 4 Julian Reschke 2011-04-10 06:51:23 PDT
Created attachment 524949 [details] [diff] [review]
implementation of exslt:date-time(), plus a minimal test case

The test case is very minimal in that it only compares the prefix of the generated string (the year), but at least the extension function gets called.
Comment 5 Jonas Sicking (:sicking) No longer reading bugmail consistently 2011-04-18 20:32:42 PDT
Use nsPrintfCString and CopyASCIItoUTF16 instead (yes, our string classes are not easy to use).

You can see how the generate-id function is implemented here:

http://mxr.mozilla.org/mozilla-central/source/content/xslt/src/xslt/txGenerateIdFunctionCall.cpp?force=1#83
http://mxr.mozilla.org/mozilla-central/source/content/xslt/src/xpath/txMozillaXPathTreeWalker.cpp#616

Also, is the string returned from this function parsable using the Date object in javascript? If so, it might be nicer to do so and check that the difference to Date.now() is less than, say, 30 minutes.
Comment 6 Julian Reschke 2011-04-19 02:05:13 PDT
(In reply to comment #5)
> Use nsPrintfCString and CopyASCIItoUTF16 instead (yes, our string classes are
> not easy to use).
> 
> You can see how the generate-id function is implemented here:
> 
> http://mxr.mozilla.org/mozilla-central/source/content/xslt/src/xslt/txGenerateIdFunctionCall.cpp?force=1#83
> http://mxr.mozilla.org/mozilla-central/source/content/xslt/src/xpath/txMozillaXPathTreeWalker.cpp#616

Will do.

> Also, is the string returned from this function parsable using the Date object
> in javascript? If so, it might be nicer to do so and check that the difference
> to Date.now() is less than, say, 30 minutes.

I tried that last week and got an exception. I didn't investigate further as the the documentation on the 8601 aspect on Date.parse is a bit weak. It would be good to know what exactly it *is* expected to parse.
Comment 8 Julian Reschke 2011-04-19 08:06:21 PDT
Created attachment 526998 [details] [diff] [review]
implementation of exslt:date-time(), plus a minimal test case
Comment 9 Julian Reschke 2011-04-19 08:07:58 PDT
(In reply to comment #6)
> (In reply to comment #5)
> > Use nsPrintfCString and CopyASCIItoUTF16 instead (yes, our string classes are
> > not easy to use).
> > 
> > You can see how the generate-id function is implemented here:
> > 
> > http://mxr.mozilla.org/mozilla-central/source/content/xslt/src/xslt/txGenerateIdFunctionCall.cpp?force=1#83
> > http://mxr.mozilla.org/mozilla-central/source/content/xslt/src/xpath/txMozillaXPathTreeWalker.cpp#616
> 
> Will do.

Done.

> > Also, is the string returned from this function parsable using the Date object
> > in javascript? If so, it might be nicer to do so and check that the difference
> > to Date.now() is less than, say, 30 minutes.
> 
> I tried that last week and got an exception. I didn't investigate further as
> the the documentation on the 8601 aspect on Date.parse is a bit weak. It would
> be good to know what exactly it *is* expected to parse.

I retried and got it working (I think I assumed wrong that Date.parse returns a Date). Set the accepted diff to 30 minutes as suggested.
Comment 10 Jonas Sicking (:sicking) No longer reading bugmail consistently 2011-04-25 18:06:16 PDT
Comment on attachment 526998 [details] [diff] [review]
implementation of exslt:date-time(), plus a minimal test case


>+            // http://exslt.org/date/functions/date-time/
>+            // format: YYYY-MM-DDTTHH:MM:SS.sss+00:00
>+            char formatstr[] = "%04hd-%02ld-%02ldT%02ld:%02ld:%02ld.%03ld%c%02ld:%02ld";
>+            size_t max = sizeof("YYYY-MM-DDTHH:MM:SS.sss+00:00");

Make this |const size_t max|.

>+  ok(now_ms - xslt_ms < accepted_diff, "generated timestamp should be not more than "
>+    + accepted_diff + " ms before 'now', but the difference was: " + (now_ms - xslt_ms));

This needs to be Math.abs(now_ms - xslt_ms), in both places.

r=me with those fixed.

If you attach a new patch (preferrably "hg export"ed) then I can land it.
Comment 11 Julian Reschke 2011-04-26 05:12:01 PDT
(In reply to comment #10)
> Comment on attachment 526998 [details] [diff] [review]
> implementation of exslt:date-time(), plus a minimal test case
> 
> 
> >+            // http://exslt.org/date/functions/date-time/
> >+            // format: YYYY-MM-DDTTHH:MM:SS.sss+00:00
> >+            char formatstr[] = "%04hd-%02ld-%02ldT%02ld:%02ld:%02ld.%03ld%c%02ld:%02ld";
> >+            size_t max = sizeof("YYYY-MM-DDTHH:MM:SS.sss+00:00");
> 
> Make this |const size_t max|.

Will do.

> >+  ok(now_ms - xslt_ms < accepted_diff, "generated timestamp should be not more than "
> >+    + accepted_diff + " ms before 'now', but the difference was: " + (now_ms - xslt_ms));
> 
> This needs to be Math.abs(now_ms - xslt_ms), in both places.

Really? isn't now_ms guaranteed to be >= xslt_ms? Anyway; I'll make that change...
Comment 12 Julian Reschke 2011-04-26 05:51:03 PDT
Created attachment 528302 [details] [diff] [review]
implementation of exslt:date-time(), plus a minimal test case
Comment 13 Julian Reschke 2011-05-03 03:12:20 PDT
Any chance to get this in before FF6 Aurora is branched?
Comment 15 Jonas Sicking (:sicking) No longer reading bugmail consistently 2011-05-09 17:05:33 PDT
Yup, checked in. Leaving the honors of marking it fixed to Julian.

(btw, the patch here contained windows lineendings which made it not apply. Easy fix though).
Comment 16 Julian Reschke 2011-05-10 00:40:15 PDT
(In reply to comment #15)
> Yup, checked in. Leaving the honors of marking it fixed to Julian.

Thanks. I build the Nightly from m-c and tested against

  http://greenbytes.de/tech/webdav/xslt-test.xml

and all is well.

> (btw, the patch here contained windows lineendings which made it not apply.
> Easy fix though).

Sorry for that.

Note You need to log in before you can comment on or make changes to this bug.