RE: [xml] RE: problem with xmlSetProp in rev 1.366


-----Original Message-----
From: Rob Richards [mailto:rrichards ctindustries net] 

Buchcik, Kasimier wrote:
Hi Rob, 

-----Original Message-----
From: Rob Richards [mailto:rrichards ctindustries net] 

Sorry I didn't catch this change earlier, but I think the 
change made to 
not add an attribute with a : in its name when no namespace 
is found is 
incorrect. The change I am referring to is rev 1.366 of tree.c:
    * If we get a QName and the prefix has no namespace-
    * binding in scope, then this is an error.
    * TODO: Previously this falled-back to non-ns handling.
    *   Should we revert this?

A colon is a legal character for an attribute name. Although 
not where 
this issue cropped up and why I didn't catch it before, 
take DOM for 
example. For DOM level 1 where there is no namespace support, the 
attribute should be added to the document with the colon 
in its name.

I think this should be reverted.

Yes, agreed.

The general problem with xmlSetProp() is that it tries to work for
both DOM levels, without actual control over it from the 
caller's side.
If I want to put in a Level 1 attribute, then I might get a Level 2
attribute back, since I happened to use a colon in my name and was
that there was a ns-decl in scope with a prefix matching 
the left side
the colon :-)
Maybe we should document very explicitely that this function should
not be used. If someone wants a pure Level 1 
attribute-setting function
than we need yet to invent one.
That might be a good idea (i.e. xmlSetNoNSProp).

Now, I was wondering about the ramifications of this 
breakage. The DOM 
example I gave was just something I thought of. The bug was actually 
caught by a PHP users trying to use ext/soap, which adds 
attributes in a 
bit of an interesting fashion, while using the latest libxml2 
release. I 
should have caught this sooner as the PHP test suite did pick up the 
failure, but I missed checking the nature of the failure.

How common do you think using a : in an attribute name without a 
matching namespace would be in apps (since this does break 
apps wether 
using the library directly or via bindings that are expecting the 
original behavior)? This is not a critical bug, but wonder if a new 
release should be made with this reverted - when Daniel gets 
back from 
holiday of course :).

Ah, a new release. Well, I broke that one, so I'm +1 for a new release
Additionally a tiny specific part of XPath (sorting) has been enhanced
recently, so the PHP user-base would be a wellcome test-team for this
enhancement ;-)



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]