Last Comment Bug 411984 - implement local-date and local-dateTime from XForms 1.1
: implement local-date and local-dateTime from XForms 1.1
Status: RESOLVED FIXED
: fixed1.8.1.13
Product: Core Graveyard
Classification: Graveyard
Component: XForms (show other bugs)
: Trunk
: x86 All
: -- normal (vote)
: ---
Assigned To: aaronr
:
Mentors:
Depends on:
Blocks: 410239
  Show dependency treegraph
 
Reported: 2008-01-11 15:10 PST by aaronr
Modified: 2016-07-15 14:46 PDT (History)
3 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
local-date() testcase (1.12 KB, application/xhtml+xml)
2008-01-11 15:18 PST, aaronr
no flags Details
local-dateTime() testcase (834 bytes, application/xhtml+xml)
2008-01-11 15:21 PST, aaronr
no flags Details
now() testcase (809 bytes, application/xhtml+xml)
2008-01-11 15:27 PST, aaronr
no flags Details
patch (8.18 KB, patch)
2008-01-11 17:14 PST, aaronr
no flags Details | Diff | Splinter Review
patch2 (10.24 KB, patch)
2008-01-11 21:19 PST, aaronr
no flags Details | Diff | Splinter Review
patch3 (11.46 KB, patch)
2008-01-13 23:33 PST, aaronr
doronr: review+
Details | Diff | Splinter Review
patch4 (12.27 KB, patch)
2008-01-14 11:22 PST, aaronr
no flags Details | Diff | Splinter Review
patch5 (12.31 KB, patch)
2008-01-14 11:47 PST, aaronr
bugs: review+
Details | Diff | Splinter Review
patch for 1.8 (17.83 KB, patch)
2008-01-15 12:04 PST, aaronr
jonas: superreview+
dveditz: approval1.8.1.13+
Details | Diff | Splinter Review
patch checked in for 1.8 (17.87 KB, patch)
2008-03-04 23:06 PST, aaronr
no flags Details | Diff | Splinter Review

Description aaronr 2008-01-11 15:10:17 PST
The W3C 'clarified' how now() is supposed to work in 1.0 to align it with 1.1 so our now() implementation is now longer 'good'.  So we really need to get now() to work according to spec and since that will give a hole w.r.t. finding the current date and time in the user's time zone, we should also implement local-date() and local-dateTime().

I had talked to msterlin about doing this work, but it looks like I've got enough free time at the moment to do it so I'll assign the work to myself.
Comment 1 aaronr 2008-01-11 15:18:50 PST
Created attachment 296605 [details]
local-date() testcase
Comment 2 aaronr 2008-01-11 15:21:03 PST
Created attachment 296607 [details]
local-dateTime() testcase
Comment 3 aaronr 2008-01-11 15:27:42 PST
Created attachment 296608 [details]
now() testcase
Comment 4 aaronr 2008-01-11 17:14:26 PST
Created attachment 296626 [details] [diff] [review]
patch

This patch implements the local-date() and local-dateTime() functions and fixes now() to get the UTC time.  This patch also removes the experimental features test around current() since 1.1 spec is now done.

Looks like nsXFormsUtils::GetDaysFromDateTime has an error in it, though, because it isn't normalizing the input parameter to UTC before returning the answer.  Also doesn't like input of the format "2008-01-11-06:00".  If I can fix these two issues relatively easily, I'll fix it in this bug since it is also time related.
Comment 5 aaronr 2008-01-11 21:19:53 PST
Created attachment 296665 [details] [diff] [review]
patch2

Fixes problem with days-from-date not handling dateTime's before the epoch correctly.
Comment 6 Doron Rosenberg (IBM) 2008-01-11 22:45:34 PST
Comment on attachment 296665 [details] [diff] [review]
patch2

>Index: nsXFormsUtils.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/xforms/nsXFormsUtils.cpp,v
>retrieving revision 1.106
>diff -u -8 -p -r1.106 nsXFormsUtils.cpp
>--- nsXFormsUtils.cpp	10 Jan 2008 23:34:08 -0000	1.106
>+++ nsXFormsUtils.cpp	12 Jan 2008 05:15:31 -0000
>@@ -2804,49 +2804,94 @@ nsXFormsUtils::GetDaysFromDateTime(const
>   NS_ASSERTION(aDays, "no return buffer for days, we'll crash soon");
>   *aDays = 0;
> 
>   PRTime date;
>   nsCOMPtr<nsISchemaValidator> schemaValidator =
>     do_CreateInstance("@mozilla.org/schemavalidator;1");
>   NS_ENSURE_TRUE(schemaValidator, NS_ERROR_FAILURE);
> 
>-  // aValue could be a xsd:date or a xsd:dateTime.  If it is a dateTime, we
>-  // should ignore the hours, minutes, and seconds according to 7.10.2 in
>-  // the spec.  So search for such things now.  If they are there, strip 'em.
>+  // aValue could be a xsd:date or a xsd:dateTime
>   PRInt32 findTime = aValue.FindChar('T');

nit - findTime is a bad variable name, as we are looking for the time separator.  Not your fault, but could be fixed.  Also would be useful to have a comment saying why we are looking for T (point at the datetime schema spec perhaps).  Someone not familiar with schema would be confused here.

> 
>-  nsAutoString dateString;
>-  dateString.Assign(aValue);
>+  nsresult rv;
>   if (findTime >= 0) {
>-    dateString.Assign(Substring(dateString, 0, findTime));
>+    rv = schemaValidator->ValidateBuiltinTypeDateTime(aValue, &date);
>+  } else {
>+    rv = schemaValidator->ValidateBuiltinTypeDate(aValue, &date);
>   }
>-
>-  nsresult rv = schemaValidator->ValidateBuiltinTypeDate(dateString, &date);
>   NS_ENSURE_SUCCESS(rv, rv);
> 
>   PRTime secs64 = date;
>   PRInt64 usecPerSec;
>   PRInt32 secs32;
> 
>   // convert from PRTime (microseconds from epoch) to seconds.  Shouldn't
>   // have to worry about remainders since input is a date.  Smallest value
>   // is in whole days.
>   LL_I2L(usecPerSec, PR_USEC_PER_SEC);
>   LL_DIV(secs64, secs64, usecPerSec);
> 
>   // convert whole seconds to PRInt32
>   LL_L2I(secs32, secs64);
> 
>-  // convert whole seconds to days.  86400 seconds in a day.
>+  // Convert whole seconds to days.  86400 seconds in a day.  If aValue
>+  // was a dateTime, "Hour, minute, and second components are ignored after
>+  // normalization" according to 7.10.2 in the spec.  Since secs32 is already
>+  // normalized, we can't just strip off hh:mm:ss since it isn't a string.  So
>+  // what we need to do is round aDays.  If secs32/86400 has a remainder,
>+  // aValue was a dateTime, and secs32 is negative, we need to decrease aDays
>+  // another day.  For example, we need to do this in order to make
>+  // 1969-01-01T01:01:01.01-06:00 produce aDays = -1.
>   *aDays = secs32/86400;
>+  if (secs32 < 0) {
>+    PRInt32 remainder = secs32%86400;
>+    if (remainder) {
>+      --*aDays;
>+    }
>+  }

This is a bit confusing - why the (secs32 < 0) check?
Comment 7 aaronr 2008-01-13 23:33:46 PST
Created attachment 296912 [details] [diff] [review]
patch3

fixed Doron's nits.  Took me a long time to come up with the new comment describing why I am looking at the remainder inside GetDaysFromDateTime.  If you don't like this comment, you'd better come up with a better one! :-)
Comment 8 aaronr 2008-01-14 11:22:36 PST
Created attachment 297008 [details] [diff] [review]
patch4

fixes issue Olli found where I wasn't handling timezones correctly in GetTime.
Comment 9 aaronr 2008-01-14 11:47:06 PST
Created attachment 297017 [details] [diff] [review]
patch5

fixing another stupid mistake I made that Olli found where I thought FindChar returned 0 if the character wasn't found but it returns kNotFound(-1) instead.  Ugh.
Comment 10 aaronr 2008-01-14 12:16:58 PST
checked into trunk
Comment 11 aaronr 2008-01-15 12:04:13 PST
Created attachment 297225 [details] [diff] [review]
patch for 1.8

Patch for 1.8 branch.  Only difference between this and trunk patch are the changes necessary for working with the 1.8 transformiix (xpath).  None of these changes to transformiix will impact any xpath user outside of xforms since they can only be reached using an xforms xpath evaluator which normal xpath doesn't use.
Comment 12 Jonas Sicking (:sicking) No longer reading bugmail consistently 2008-02-29 19:56:32 PST
Comment on attachment 297225 [details] [diff] [review]
patch for 1.8

Looks good. Really sorry about the long delay :(
Comment 13 Daniel Veditz [:dveditz] 2008-03-04 10:42:49 PST
Comment on attachment 297225 [details] [diff] [review]
patch for 1.8

approved for 1.8.1.13, a=dveditz for release-drivers
Comment 14 aaronr 2008-03-04 23:06:26 PST
Created attachment 307412 [details] [diff] [review]
patch checked in for 1.8

this is the patch I checked in for 1.8.  Same as last patch, just updated to the branch.

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