284351 - nsSchemaLoader doesn't handle xsd:complexType correctly.

Status: RESOLVED FIXED

(Core Graveyard :: Web Services, defect)

Description

[Doron Rosenberg (IBM)]

So I have a schema file with:

	<xsd:complexType name="MoneyLaundering">
		<xsd:annotation>
			<xsd:documentation>Electronic Money Laundering</xsd:documentation>
		</xsd:annotation>
		<xsd:sequence maxOccurs="unbounded">
			<xsd:element ref="ml:individual"/>
		</xsd:sequence>
	</xsd:complexType>

 http://lxr.mozilla.org/seamonkey/source/extensions/webservices/schema/src/nsSchemaLoader.cpp#1305
is called on the annotation node, and
http://lxr.mozilla.org/seamonkey/source/extensions/webservices/schema/src/nsDOMUtils.h#119
results in the mathml namespace.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Where does nsMathMLAtoms come in?  Is there a testcase that shows the bug?

Note that there is a nsMathMLAtoms::annotation_ atom that maps to the string
"annotation".  But that would be the tagname, not the namespace here...

Comment 2

[Doron Rosenberg (IBM)]

I'm working on a testcase.  It seems the DOMElement for the xsd:annotation node
uses nsMathMLAtoms:Annotation for its mName.

I thought this was failing:
http://lxr.mozilla.org/seamonkey/source/extensions/webservices/schema/src/nsSchemaLoader.cpp#1323
since we seem to get into the last else{} but I am told that compating atoms who
define "annotation" should be equal.  I'll find out more tomorrow.

This bug was more of a reminder for me on what I found out and to include in my
status report :)

Comment 3

[Boris Zbarsky [:bzbarsky]]

> It seems the DOMElement for the xsd:annotation node uses
> nsMathMLAtoms:Annotation for its mName.

That's fine, and the expected behavior under the circumstances.  Why is this a
problem?

Comment 4

[Doron Rosenberg (IBM)]

Attachment: Correctly follow the schema spec

Comment 5

[Doron Rosenberg (IBM)]

Attachment: Follow the spec, with more diff context.

Comment 6

[Doron Rosenberg (IBM)]

Spec is here:
http://w3.org/TR/xmlschema-1/#element-complexType

Oops, patch had tabs, going to fix that in a sec.

Comment 7

[Doron Rosenberg (IBM)]

Attachment: remove tabs.

Comment 8

[Doron Rosenberg (IBM)]

The problem here was simple - if we hit annotation, we would bail, saying its
invalid, while all other items were processed:

  else if (tagName != nsSchemaAtoms::sAnnotation_atom) {
  } else {
  }

This patch follows the spec and bails on any item not allowed in the spec,
rather than trying to process ones we know aren't allowed.  Also, it handles
annotations and currently does nothing with them.

Comment 9

[Peter Van der Beken [:peterv]]

Comment on attachment 176050 [details] [diff] [review]
remove tabs.

You could remove all the else-after-break's too. But shouldn't we actually
continue looping after finding an annotation? With this patch we'll ignore the
simpleContent/complexContent/... coming after the annotations.

Comment 10

[Doron Rosenberg (IBM)]

Ah, so just removing the break; from the annotation condition should do it, right?

Comment 11

[Doron Rosenberg (IBM)]

Attachment: don't break when we find an annotation

Comment 12

[Peter Van der Beken [:peterv]]

Comment on attachment 176737 [details] [diff] [review]
don't break when we find an annotation

Sure, but if you can, please remove the else's after a break, they're not
needed.

Comment 13

[Doron Rosenberg (IBM)]

checked in.