Re: [xml] Potential wrong usage of xmlIsID() in tree.c

[Rob Richards <rrichards ctindustries net>]

Kasimier Buchcik wrote:
> Hi,
>
> On Tue, 2006-02-21 at 16:06 +0100, Kasimier Buchcik wrote:
>   
> > Hi,
> >
> > On Tue, 2006-02-21 at 08:56 -0500, Rob Richards wrote:
> >     
> > > Kasimier Buchcik wrote:
> > >       
> > > > > As far as copy/import/adopt goes. I have found a couple of conflicting 
> > > > > opinions for this. One is that the importing doc should determine if an 
> > > > > attribute is added as an ID. The other (which is also how it is 
> > > > > implemented in Xerces) is that an attribute should be copied/imported 
> > > > > with the same ID-ness as in the source document. The latter would be in 
> > > > >     
> > > > >           
> > > > I agree with the latter mechanism. This looks like the only
> > > > sane/efficient mechanism to me currently (this is also the reason why
> > > > I'm not confident with automatic IDness detection).
> > > >   
> > > >         
> > > I agree on the latter method as well. I do see some potential issues 
> > > with the automatic detection after thinking about it more. From a purely 
> > > DTD view, if the document is parsed without validation or attribute 
> > > handling enabled, the current implementation will still create IDs 
> > > modiying the tree. There is no indication in the document that
> > > this was not desired. So the question comes to mind should IDs be 
> > > created or not?
> > >       
> > Ah, good point. I think it should not automatically create IDs in this
> > case, since we don't know if the attribute's element is valid, and thus
> > if the element/attribute is valid at that position of the tree. So we
> > could run into creating IDness for attributes which wouldn't become IDs
> > if processed by the validator.
> >     
>
> I noticed that this is actually not an answer for your question :-) but
> writing this down had the nice side-effect that I realized that the
> current detection of IDness could mark attributes as IDs, even if they
> are not IDs, but invalid attributes according to a DTD; i.e., to query
> the DTD for an element/attribute combination is not enough to evaluate
> if an attribute is an ID. So one could argue that the existing detection
> is a bug.
>   
How do you figure (or are you referring to the case of a document not 
parsed in validating mode)? A DTD doesn't allow a redefinition of an 
element and an element can have only a single ID defined, so the query 
for an element/attr combo should be enough. XML Schemas would be a whole 
different story though.
> To your question: Ah, good point :-) I think this should either be made
> settable or be avoided. The latter being the solution I would prefer.
>   
I haven't scoured the XInclude code, but when a copy is being performed 
from there, has the attribute being copied already been determined to be 
an ID? If so, I would not be adverse to having the copied attribute an 
ID as well in all cases. I assume that the ID behavior when copying an 
attribute is not going to be removed due to the use from XInclude, but 
if the atype could be tested rather than xmlIsID, then the negative 
impact would at least be less than it currently is.

When creating a new property is it possible that an ID check is only 
performed if the attribute is being created in the scope of an element 
and NULL is passed as the parent element to xmlIsID? This would at least 
only create an ID if it is a proper xml:id.

The setprop function should be fine. I have a scaled down patch almost 
ready that addresses that function correctly.

Other than completely removing automatic detection after the fact, I 
think this may be the best bet for now. Anything more looks like will 
require a bit of refactoring and probably a good bit of discussion.

Rob