Status

RESOLVED FIXED
14 years ago
2 years ago

People

(Reporter: doronr, Assigned: doronr)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

14 years ago
I think xforms should have a nsXFormsSchemaValidator class, that internally has
a nsISchemaValidator.  nsXFormsSchemaValidator would use it for everything other
than items in the XForms namespace, which would be handled in that file, as
XForms has seveal schema types it defines in its namespace.  Sound ok?

Updated

14 years ago
OS: Windows XP → All
Hardware: PC → All
(Assignee)

Comment 1

14 years ago
Created attachment 168643 [details] [diff] [review]
shim class
Assignee: aaronr → doronr
Status: NEW → ASSIGNED
(Assignee)

Comment 2

14 years ago
Sample code:

 nsCOMPtr<nsISchemaLoader> schemaLoader =
do_GetService("@mozilla.org/xmlextras/schemas/schemaloader;1");
 nsCOMPtr<nsISchema> schema;
   schemaLoader->Load(NS_LITERAL_STRING("schea.xsd"), getter_AddRefs(schema));

 nsXFormsSchemaValidator *xformsValidator;
 NS_NewXFormsSchemaValidator(&xformsValidator);
 t->LoadSchema(schema);

 PRBool foo = t->ValidateString(NS_LITERAL_STRING("123"),
NS_LITERAL_STRING("integer"),
NS_LITERAL_STRING("http://www.w3.org/1999/XMLSchema"));

(Assignee)

Comment 3

14 years ago
Comment on attachment 168643 [details] [diff] [review]
shim class

I am not sure if I need to addref in the NS_NewXFormsSchemaValidator method or
not. Do I?
Attachment #168643 - Flags: superreview?(darin)
(Assignee)

Updated

14 years ago
Attachment #168643 - Attachment is obsolete: true
Attachment #168643 - Flags: superreview?(darin)
(Assignee)

Comment 4

14 years ago
Created attachment 168647 [details] [diff] [review]
shim with darin's review comments fixed.
Attachment #168647 - Flags: superreview?(darin)

Comment 5

14 years ago
Comment on attachment 168647 [details] [diff] [review]
shim with darin's review comments fixed.

>+nsXFormsSchemaValidator::nsXFormsSchemaValidator()
>+{
>+  mSchemaValidator = do_GetService(NS_SCHEMAVALIDATOR_CONTRACTID);
>+}
>+
>+nsresult nsXFormsSchemaValidator::LoadSchema(nsISchema* aSchema)
>+{
>+  nsresult rv = mSchemaValidator->LoadSchema(aSchema);

If do_GetService returns null because for some reason the schema
validation extension could not be found, then LoadSchema will crash.
You should null check that.



>+  NS_ENSURE_SUCCESS(rv, rv);
>+
>+  return rv;

NS_ENSURE_SUCCESS(rv, rv); expands to |if (NS_FAILED(rv)) return rv;|, so
here you are basically writing:

  if (NS_FAILED(rv))
    return rv;
  return rv;

Seems like that could be simplified :-)

I wouldn't bother outputing a warning when LoadSchema fails since you
are propogating that error code to the caller.	Instead, I would output
a warning if mSchemaValidator is null.
Attachment #168647 - Flags: superreview?(darin) → superreview-
(Assignee)

Comment 6

14 years ago
Created attachment 168663 [details] [diff] [review]
more nit fixing.
Attachment #168647 - Attachment is obsolete: true
Attachment #168663 - Flags: superreview?(darin)

Comment 7

14 years ago
Comment on attachment 168663 [details] [diff] [review]
more nit fixing.

sr=darin

looks good to me, but please hold off on checking this patch in until
extensions/schema-validation compiles on the trunk without additional patches. 
there is a tinderbox building xforms! ;-)
Attachment #168663 - Flags: superreview?(darin) → superreview+

Updated

14 years ago
Blocks: 278762
(Assignee)

Comment 8

14 years ago
fixed
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.