Note: There are a few cases of duplicates in user autocompletion which are being worked on.

implement local-date and local-dateTime from XForms 1.1

RESOLVED FIXED

Status

Core Graveyard
XForms
RESOLVED FIXED
10 years ago
a year ago

People

(Reporter: aaronr, Assigned: aaronr)

Tracking

({fixed1.8.1.13})

Trunk
x86
All
fixed1.8.1.13

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 5 obsolete attachments)

(Assignee)

Description

10 years ago
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.
(Assignee)

Comment 1

10 years ago
Created attachment 296605 [details]
local-date() testcase
(Assignee)

Comment 2

10 years ago
Created attachment 296607 [details]
local-dateTime() testcase
(Assignee)

Comment 3

10 years ago
Created attachment 296608 [details]
now() testcase
(Assignee)

Comment 4

10 years ago
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.
(Assignee)

Updated

10 years ago
Blocks: 410239
(Assignee)

Comment 5

10 years ago
Created attachment 296665 [details] [diff] [review]
patch2

Fixes problem with days-from-date not handling dateTime's before the epoch correctly.
Attachment #296626 - Attachment is obsolete: true
Attachment #296665 - Flags: review?(doronr)
(Assignee)

Updated

10 years ago
Attachment #296665 - Flags: review?(Olli.Pettay)

Comment 6

10 years ago
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?
(Assignee)

Comment 7

10 years ago
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! :-)
Attachment #296665 - Attachment is obsolete: true
Attachment #296665 - Flags: review?(doronr)
Attachment #296665 - Flags: review?(Olli.Pettay)
(Assignee)

Updated

10 years ago
Attachment #296912 - Flags: review?(doronr)
(Assignee)

Updated

10 years ago
Attachment #296912 - Flags: review?(Olli.Pettay)

Updated

10 years ago
Attachment #296912 - Flags: review?(doronr) → review+
(Assignee)

Comment 8

10 years ago
Created attachment 297008 [details] [diff] [review]
patch4

fixes issue Olli found where I wasn't handling timezones correctly in GetTime.
Attachment #296912 - Attachment is obsolete: true
Attachment #297008 - Flags: review?(Olli.Pettay)
Attachment #296912 - Flags: review?(Olli.Pettay)
(Assignee)

Comment 9

10 years ago
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.
Attachment #297008 - Attachment is obsolete: true
Attachment #297017 - Flags: review?(Olli.Pettay)
Attachment #297008 - Flags: review?(Olli.Pettay)

Updated

10 years ago
Attachment #297017 - Flags: review?(Olli.Pettay) → review+
(Assignee)

Comment 10

10 years ago
checked into trunk
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Whiteboard: xf-to-branch
(Assignee)

Comment 11

10 years ago
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.
Attachment #297225 - Flags: review?(jonas)
(Assignee)

Updated

10 years ago
Attachment #297225 - Flags: review?(jonas) → superreview?(jonas)
Comment on attachment 297225 [details] [diff] [review]
patch for 1.8

Looks good. Really sorry about the long delay :(
Attachment #297225 - Flags: superreview?(jonas) → superreview+
(Assignee)

Updated

10 years ago
Attachment #297225 - Flags: approval1.8.1.13?
Comment on attachment 297225 [details] [diff] [review]
patch for 1.8

approved for 1.8.1.13, a=dveditz for release-drivers
Attachment #297225 - Flags: approval1.8.1.13? → approval1.8.1.13+
(Assignee)

Comment 14

10 years ago
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.
Attachment #297225 - Attachment is obsolete: true
(Assignee)

Updated

10 years ago
Keywords: fixed1.8.1.13
Whiteboard: xf-to-branch
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.