328092 - SchemaValidation/Durations have problems with pre-1900 dates due to NSPR

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[Doron Rosenberg (IBM)]

Comment 1

[Doron Rosenberg (IBM)]

Attachment: patch

Comment 2

[aaronr]

Comment on attachment 214483 [details] [diff] [review]
patch


Please use -p in your cvs diffs so it is easier to figure out the functions where your changes are.

Another general comment that is widespread in this patch -> why not just have one function like this: 
static PRBoolIsValidSchemaInteger(const nsAString & aNodeValue, long *aResult, PRBool aOverFlowCheck = PR_FALSE);

Then you don't have to go through a forwarder when it only has two parameters and you pass in the PR_FALSE in explicitly.

Another general comment: I don't see where you test for aNodeValue existing or being a string of any length at all when you are checking to see if aNodeValue is a valid type of something.  For most of these couldn't you bow out early if aNodeValue wasn't there or of 0 length?

>Index: extensions/schema-validation/src/nsSchemaValidator.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/schema-validation/src/nsSchemaValidator.cpp,v
>retrieving revision 1.20
>diff -u -r1.20 nsSchemaValidator.cpp
>--- extensions/schema-validation/src/nsSchemaValidator.cpp	21 Feb 2006 22:01:13 -0000	1.20
>+++ extensions/schema-validation/src/nsSchemaValidator.cpp	8 Mar 2006 23:10:42 -0000
>@@ -2081,26 +2084,17 @@
>                                            PRBool *aResult)
> {
>   PRBool isValid = PR_FALSE;
>-  PRTime dateTime;
>-  PRExplodedTime explodedTime;
>+  nsSchemaTime time;
>   int timeCompare;
> 
>-  isValid = IsValidSchemaTime(aNodeValue, &dateTime);
>-
>-  if (isValid) {  
>-    // convert it to a PRExplodedTime 
>-    PR_ExplodeTime(dateTime, PR_GMTParameters, &explodedTime);
>-  }
>+  isValid = IsValidSchemaTime(aNodeValue, &time);

Why not just declare isValid to be a PRBool here instead of assigning it PR_FALSE to begin with?


>@@ -721,14 +693,98 @@
> }
> 
> void
>-nsSchemaValidatorUtils::GetMonthShorthand(char* aMonth, nsACString & aReturn)
>+nsSchemaValidatorUtils::AddTimeZoneToDateTime(nsSchemaDateTime aDateTime,
>+                                              nsSchemaDateTime* aDestDateTime)
> {

>+
>+  printf("\n ::: %u:%d:%d %d:%d:%d\n", year, month, day, hour, minute, second);
>+

nit: make printf only during debug or surround with LOG.


>   PRBool done = PR_FALSE;
>   while (!done) {
>-    maxDay = GetMaximumDayInMonthFor(resultDatetime.tm_year,
>-                                         resultDatetime.tm_month);
>-    if (resultDatetime.tm_mday < 1) {
>-      resultDatetime.tm_mday +=
>-        GetMaximumDayInMonthFor(resultDatetime.tm_year,
>-                                resultDatetime.tm_month - 1);
>+    maxDay = GetMaximumDayInMonthFor(aResultDateTime->date.year,
>+                                         aResultDateTime->date.month);

nit: align parameters, please.

>Index: extensions/schema-validation/src/nsSchemaValidatorUtils.h
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/schema-validation/src/nsSchemaValidatorUtils.h,v
>retrieving revision 1.7
>diff -u -r1.7 nsSchemaValidatorUtils.h
>--- extensions/schema-validation/src/nsSchemaValidatorUtils.h	21 Feb 2006 22:01:13 -0000	1.7
>+++ extensions/schema-validation/src/nsSchemaValidatorUtils.h	8 Mar 2006 23:10:43 -0000


>-  static PRBool GetPRTimeFromDateTime(const nsAString & aNodeValue, PRTime *aResult);
>+  static int CompareDateTime(nsSchemaDateTime aDateTime1,
>+                                     nsSchemaDateTime aDateTime2);

nit: align parameters, please.

with these, r=me

Comment 3

[Doron Rosenberg (IBM)]

fixed on trunk

Comment 4

[Doron Rosenberg (IBM)]

checked into 1.8.1

Comment 5

[Doron Rosenberg (IBM)]

checked into 1.8.0.3