411984 - implement local-date and local-dateTime from XForms 1.1

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[aaronr]

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]

Attachment: local-date() testcase

Comment 2

[aaronr]

Attachment: local-dateTime() testcase

Comment 3

[aaronr]

Attachment: now() testcase

Comment 4

[aaronr]

Attachment: 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]

Attachment: patch2

Fixes problem with days-from-date not handling dateTime's before the epoch correctly.

Comment 6

[Doron Rosenberg (IBM)]

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]

Attachment: 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]

Attachment: patch4

fixes issue Olli found where I wasn't handling timezones correctly in GetTime.

Comment 9

[aaronr]

Attachment: 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]

checked into trunk

Comment 11

[aaronr]

Attachment: 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]

Comment on attachment 297225 [details] [diff] [review]
patch for 1.8

Looks good. Really sorry about the long delay :(

Comment 13

[Daniel Veditz [:dveditz]]

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]

Attachment: 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.