invalid error message using inline schemas: Ignoring dup schema

RESOLVED FIXED

Status

Core Graveyard
XForms
--
minor
RESOLVED FIXED
11 years ago
a year ago

People

(Reporter: Steve Speicher, Assigned: Merle Sterling)

Tracking

({fixed1.8.0.12, fixed1.8.1.4})

Trunk
x86
Windows XP
fixed1.8.0.12, fixed1.8.1.4

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

1.30 KB, application/xhtml+xml
Details
1.82 KB, patch
aaronr
: review+
smaug
: review+
Details | Diff | Splinter Review
(Reporter)

Description

11 years ago
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
(Reporter)

Comment 1

11 years ago
Created attachment 235266 [details]
testcase
(Assignee)

Comment 2

11 years ago
Created attachment 245599 [details] [diff] [review]
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.
Attachment #245599 - Flags: review?(aaronr)

Updated

11 years ago
Assignee: xforms → msterlin
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 3

11 years ago
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?
Attachment #245599 - Flags: review?(aaronr) → review-

Comment 4

11 years ago
(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

11 years ago
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.
(Assignee)

Comment 6

11 years ago
(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.



(Assignee)

Comment 7

11 years ago
Created attachment 245847 [details] [diff] [review]
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.
Attachment #245599 - Attachment is obsolete: true
(Assignee)

Updated

11 years ago
Attachment #245847 - Flags: review?(aaronr)

Updated

11 years ago
Attachment #245847 - Flags: review?(aaronr)
Attachment #245847 - Flags: review?(Olli.Pettay)
Attachment #245847 - Flags: review+

Updated

11 years ago
Attachment #245847 - Flags: review?(Olli.Pettay) → review+

Comment 8

11 years ago
checked into trunk for msterlin
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Whiteboard: xf-to-branch

Comment 9

10 years ago
checked into 1.8 branch on 2007-04-12
checked into 1.8.0 branch on 2007-04-16
Keywords: fixed1.8.0.12, fixed1.8.1.4
Whiteboard: xf-to-branch
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.