Closed Bug 274083 Opened 20 years ago Closed 19 years ago

nsXFormsSchemaValidator

Categories

(Core Graveyard :: XForms, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: doronr, Assigned: doronr)

References

Details

Attachments

(1 file, 2 obsolete files)

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?
OS: Windows XP → All
Hardware: PC → All
Attached patch shim class (obsolete) — Splinter Review
Assignee: aaronr → doronr
Status: NEW → ASSIGNED
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"));

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)
Attachment #168643 - Attachment is obsolete: true
Attachment #168643 - Flags: superreview?(darin)
Attachment #168647 - Flags: superreview?(darin)
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-
Attached patch more nit fixing.Splinter Review
Attachment #168647 - Attachment is obsolete: true
Attachment #168663 - Flags: superreview?(darin)
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+
Blocks: 278762
fixed
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: