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

[Rob Richards <rrichards ctindustries net>]

Hi Kasimier,

Won't be able to go over some of the things you brought up in detail 
until tomorrow, so here's an abbreviated response on some of the issues.

Kasimier Buchcik wrote:

> > It never even crossed my mind that someone might be building a list of 
> > attributes without a corresponding element. Have no qualms about 
> > removing that check.
> >    
>
> I really dunno; the attribute handling is this function looks weird
> anyway, so I could think of that one being used for weird use-cases.
> But I have no preferences here, since I don't use it.
>  
It could probably be left as is and allow for an attribute without 
parent. I also have no preference for the same reason, but if anyone is 
using those functions with attributes, the potential crashes needs to be 
fixed at a minimum.

>  
>
> > >  
> > >  b) You noticed the potential chance here of freeing
> > >    the attribute (@cur) to which a sibling attribute (@elem) should be
> > >    attached if both have the same ns and local-name. We need to fix
> > >    that.
> > >  
> > >      
> > Yup, ran across this issue in all the sibling functions.
> >    
>
> OK. What do you think about compiling this semantic and
> also the addition/removal of IDs into one function to be called by the
> various append-something-to-something functions? Or would it slow down
> things too much?
>  
I was planning on trying to do that as part of my cleanup. I think for 
maintainability it needs to be done more than anything.
Really wanted to just show the proposed ID changes in that patch.

>  
>
> > > 2) xmlAddPrevSibling():
> > >  a) Similar to xmlAddNextSibling().
> > >  b) Tries to automatically convert the doc of @elem, if different.
> > >    I fear that trying to handle moving-to-different-docs related
> > >    semantics at the level of every tree-modifying function will
> > >    result in much scattered and potentially hard-to-maintain code.
> > >      
> > I agree with this and personally handle this myself prior to calling the 
> > functions, but the functionality for this has always been here so could 
> > possibly break apps if removed. All I did here was move it around a bit.
> >    
>
> Ah, OK. So it even needs to take care of moving the value to the
> destination doc's dict if it was in the source doc's dict.
>  
You just hit on the issue with xmlSetTreeDoc. Basically it needs to 
incorporate the adopt string functionality you wrote with the DOM 
functions. I had started looking at this a while back but didnt have the 
time and evenutally forgot about this until now :(

> > > About automatic ID detection:
> > >  IIRC, for DOM, there's no such automatic behaviour defined.
> > >  
> > >      
> > This would probably fall under the category of:
> > "using mechanisms that are outside the scope of this specification, it 
> > is then an externally-determined ID attribute."
> >    
>
> Hmm, I don't think so, since for DTD, XML Schema and setting IDs via the
> API, this is defined (directly above that specific sentence in the
> spec). I think this is intended to be a fall-back rule for other
> mechanisms.
>  
I'm not sure either. Was throwing it out as a possible answer since we 
all know how clear the DOM specs always are :)

> OK. The first important thing for me to know is, if we can revert the
> already existent automatic detection of IDness via xmlIsID() or not.
>  
I believe it was added in .21. I didn't give it much thought until you 
brought this issue up, but have no problems if it gets removed until 
everything gets hashed out. This is just my opinion as I am not sure how 
many apps are relying on this functionality now.

Rob