278447 - Implement XForms Schema Types

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[Doron Rosenberg (IBM)]

In nsXFormsSchemaValidator we need to split out xforms schema types and handle
them seperatly.

Comment 1

[aaronr]

*** Bug 279026 has been marked as a duplicate of this bug. ***

Comment 2

[Doron Rosenberg (IBM)]

taking

Comment 3

[Doron Rosenberg (IBM)]

Attachment: patch

Comment 4

[Doron Rosenberg (IBM)]

Attachment: testcase

Comment 5

[aaronr]

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


>-PRBool nsXFormsSchemaValidator::ValidateString(const nsAString & aValue,
>-  const nsAString & aType, const nsAString & aNamespace)
>+PRBool
>+nsXFormsSchemaValidator::ValidateString(const nsAString & aValue,
>+                                        const nsAString & aType,
>+                                        const nsAString & aNamespace)
> {
>   PRBool isValid = PR_FALSE;
> 
>   NS_ENSURE_TRUE(mSchemaValidator, isValid);
>-  mSchemaValidator->ValidateString(aValue, aType, aNamespace, &isValid);
>+
>+  // if it is the XForms namespace, handle it internally, else delegate to
>+  // nsISchemaValidator
>+  if (aNamespace.EqualsLiteral(NS_NAMESPACE_XFORMS)) {
>+    isValid = VaidateXFormsTypeString(aValue, aType);
>+  } else {
>+    mSchemaValidator->ValidateString(aValue, aType, aNamespace, &isValid);
>+  }
> 
>   return isValid;
> }

VaidateXFormsTypeString?  Really?  At least you are consistent across the file,
but please
correct spelling of 'Validate'

> 
>-PRBool nsXFormsSchemaValidator::GetType(const nsAString & aType,
>-  const nsAString & aNamespace, nsISchemaType **aSchemaType)
>+PRBool
>+nsXFormsSchemaValidator::GetType(const nsAString & aType,
>+                                 const nsAString & aNamespace,
>+                                 nsISchemaType **aSchemaType)
> {
>   NS_ENSURE_TRUE(mSchemaValidator, PR_FALSE);
>-  nsresult rv = mSchemaValidator->GetType(aType, aNamespace, aSchemaType);
>+  PRBool success = PR_FALSE;
>+
>+  if (!aNamespace.EqualsLiteral(NS_NAMESPACE_XFORMS)) {
>+    nsresult rv = mSchemaValidator->GetType(aType, aNamespace, aSchemaType);
>+    success = NS_SUCCEEDED(rv);
>+  }
>+
>+  return success;
>+}

Why do you automatically return false if XForms type?  Please comment.
Nit: Also, please keep the style consistent, set success = PR_FALSE
and use NS_ENSURE_TRUE(mSchemaValidator, success) like in your other
functions.

>+
>+// xforms schema types
>+PRBool
>+nsXFormsSchemaValidator::VaidateXFormsTypeString(const nsAString & aValue,
>+                                                 const nsAString & aType)
>+{

Please correct spelling of 'Validate'


>+
>+PRBool
>+nsXFormsSchemaValidator::IsValidSchemaListItem(const nsAString & aValue)
>+{
>+  PRBool isValid = PR_FALSE;
>+
>+  // like a string, but no whitespace
>+  if (aValue.FindChar(' ') == kNotFound) {
>+    mSchemaValidator->ValidateString(aValue, NS_LITERAL_STRING("string"),
>+                                     NS_LITERAL_STRING("http://www.w3.org/1999/XMLSchema"),
>+                                     &isValid);
>+  }
>+
>+  return isValid;
>+}

Is that a good test for whitespace?  What about tabs?  Linefeed? 
nsXFormsXPathXMLUtil::IsWhitespace
seems more thorough, for instance. 


>+
>+PRBool
>+nsXFormsSchemaValidator::IsValidSchemaListItems(const nsAString & aValue)
>+{
>+  PRBool isValid = PR_FALSE;
>+
>+  // listItem is like a string, but no whitespace.  listItems is a whitespace
>+  // delimated lsit of listItem, so therefore just need to see if it is a valid
>+  // xsd:string
>+  mSchemaValidator->ValidateString(aValue, NS_LITERAL_STRING("string"),
>+                                   NS_LITERAL_STRING("http://www.w3.org/1999/XMLSchema"),
>+                                   &isValid);
>+
>+  return isValid;
>+}

Nit: correct spelling in comment

>+
>Index: extensions/xforms/nsXFormsSchemaValidator.h
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/xforms/nsXFormsSchemaValidator.h,v
>retrieving revision 1.5
>diff -u -r1.5 nsXFormsSchemaValidator.h
>--- extensions/xforms/nsXFormsSchemaValidator.h	22 Feb 2005 22:11:51 -0000	1.5
>+++ extensions/xforms/nsXFormsSchemaValidator.h	28 Jun 2005 19:18:10 -0000
>@@ -47,11 +47,20 @@
> 
>   nsresult LoadSchema(nsISchema* aSchema);
>   PRBool ValidateString(const nsAString & aValue, const nsAString & aType,
>-    const nsAString & aNamespace);
>-  PRBool Validate(nsIDOMNode* aElement);
>+                        const nsAString & aNamespace);
>+  PRBool ValidateNode(nsIDOMNode* aElement, const nsAString & aType,
>+                      const nsAString & aNamespace);
>   PRBool GetType(const nsAString & aType, const nsAString & aNamespace,
>                  nsISchemaType **aSchemaType);
>-    
>+
>+  // xforms schema types
>+  PRBool VaidateXFormsTypeString(const nsAString & aValue,
>+                                 const nsAString & aType);
>+
>+  PRBool IsValidSchemaYearMonthDuration(const nsAString & aValue);
>+  PRBool IsValidSchemaDayTimeDuration(const nsAString & aValue);
>+  PRBool IsValidSchemaListItem(const nsAString & aValue);
>+  PRBool IsValidSchemaListItems(const nsAString & aValue);
> 
> protected:
>   nsCOMPtr<nsISchemaValidator> mSchemaValidator;

Correct spelling in 'Validate'.  

Also, you don't have a single comment in nsXFormsSchemaValidator.h or
nsXFormsSchemaValidator.cpp as to what any of these functions do, who might
call them, where they fit in the overall scheme of things or why this class
even exists.  Give us SOMETHING, please.

Comment 6

[Doron Rosenberg (IBM)]

Attachment: better patch

Comment 7

[Doron Rosenberg (IBM)]

Comment on attachment 187662 [details] [diff] [review]
better patch

wrong patch, sorry

Comment 8

[Doron Rosenberg (IBM)]

Attachment: patch with comments addressed

Comment 9

[Olli Pettay [:smaug][[e-mail]] (holiday -> Aug30)]

Comment on attachment 187667 [details] [diff] [review]
patch with comments addressed


>+PRBool
>+nsXFormsSchemaValidator::ValidateXFormsTypeString(const nsAString & aValue,
>+                                                 const nsAString & aType)

Add space before const nsAString & aType



>+    // if years/months exist, invalid
>+    if ((years == 0) && (months == 0))
>+      isValid = PR_TRUE;

isValid = (years == 0) && (months == 0);


>+  PRBool ValidateXFormsTypeString(const nsAString & aValue,
>+                                 const nsAString & aType);

Add space before const nsAString & aType


With those r=me

Comment 10

[Doron Rosenberg (IBM)]

checked in.