350067 - invalid error message using inline schemas: Ignoring dup schema

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[Steve Speicher]

User-Agent:       Opera/9.00 (Windows NT 5.1; U; en)
Build Identifier: 

I only have this schema defined once but I'm getting an invalid error.  Note, this doesn't affect the form processing as it just ignores it.

It appears that nsXFormsModelElement::IsDuplicateSchema() is invoked both from core processing of model @schema within nsME::InitializeInstance() and then later from nsME::FinishConstruction().  It appears that FinishConstruction() doesn't take into account mSchemaTotal.

Error received:
XForms Error (40): Ignoring duplicate schema with target namespace: http://example.com

Reproducible: Always

Comment 1

[Steve Speicher]

Attachment: testcase

Comment 2

[Merle Sterling]

Attachment: patch

During nsxFormsModelElement::InitializeInstances the inline schema referenced by the schema attribute on the model element is found and processed but FinishConstruction then finds the xsd:schema element and processes it again. So it is considered a duplicate because it has already been processed - not because it is actually another schema with the same targetNamespace.

If we simply check that mSchemaCount (number of schemas already processed) == mSchemaTotal we avoid the duplicate processing of the same schema.

I verified that the duplicate schema error message (which is valid) still occurs for the testcases in bug 299173 and not for this testcase where it would be invalid.

Comment 3

[aaronr]

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

mSchemaTotal is the number of schemas mentioned in @schema on the xf:model.  mSchemaCount is the number of processed schemas.  They could be equal and still have inline schema to process.  For example, I believe that your fix would cause the example at: file:///c:/xforms/spec/index-all.html#concepts-model to not have its inline schema processed, right?

Comment 4

[aaronr]

(In reply to comment #3)
> (From update of attachment 245599 [details] [diff] [review] [edit])
> mSchemaTotal is the number of schemas mentioned in @schema on the xf:model. 
> mSchemaCount is the number of processed schemas.  They could be equal and still
> have inline schema to process.  For example, I believe that your fix would
> cause the example at: file:///c:/xforms/spec/index-all.html#concepts-model to
> not have its inline schema processed, right?
> 

Sorry, I guess I shouldn't have given the URI of my local copy of the spec :-)  Try this instead: http://www.w3.org/TR/xforms/slice2.html#concepts-model

Comment 5

[aaronr]

I think the problem is that we are processing schema fragments that live inside the model during InitializeInstance and we probably shouldn't be.  Look here: http://www.w3.org/TR/xforms/slice3.html#structure-model, it says this about the schema attribute on the model element: "Optional list of xsd:anyURI links to XML Schema documents outside this model element".

So I'm guessing that if we find that one of the values in @schema lives in the current document (which we already test for) then we need to see if it lives inside the model element and if it does, ignore it.  Then we'll take care of it inside FinishConstruction.

Comment 6

[Merle Sterling]

(In reply to comment #5)
> I think the problem is that we are processing schema fragments that live inside
> the model during InitializeInstance and we probably shouldn't be.  Look here:
> http://www.w3.org/TR/xforms/slice3.html#structure-model, it says this about the
> schema attribute on the model element: "Optional list of xsd:anyURI links to
> XML Schema documents outside this model element".
> So I'm guessing that if we find that one of the values in @schema lives in the
> current document (which we already test for) then we need to see if it lives
> inside the model element and if it does, ignore it.  Then we'll take care of it
> inside FinishConstruction.

That's one way to do it and was considered but I am always leery of removing code that already exists.  Another more painful way is to maintain another hashtable that maps targetNamespace to schema ID and checking if we have already processed that particular ID. I abandoned the second approach because it is way more confusing. I thought this might for once be a 'simple' oversight that could be fixed with one line of code. :)

Note that the comment in FinishConstruction says 'process schemas that aren't referenced by the schema attribute' - but it does NOT do that because the testcase is an example of a schema referenced by the schema attribute and it was already processed once during InitializeInstances.

Comment 7

[Merle Sterling]

Attachment: patch

This patch takes the simple approach: if we find an inline schema within the model element that was referenced by the schema attribute then we skip it so it is not processed twice. It will be processed in FinishConstruction.

The alternative approach would be to create a new hashtable to map targetNamespace to schema elements for every schema element that we process (something like mProcessedInlineSchemas).  We would then have to add each schema that is processed to the hashtable and check the hashtable in FinishConstruction so we don't process the same schema again.  That approach is easy to understand but introduces a lot more overhead than the simple approach in the first patch.

Comment 8

[aaronr]

checked into trunk for msterlin

Comment 9

[aaronr]

checked into 1.8 branch on 2007-04-12
checked into 1.8.0 branch on 2007-04-16