>    public final class NodeRevisionDescriptor implements Serializable, Cloneable {
> 
>   @@ -542,7 +542,7 @@
>         * @param name New name
>         */
>        public void setName(String name) {
>   -        setProperty(NAME, name, true);
>   +        setProperty(NAME, "<![CDATA[" + name + "]]>", true);
>        }

Remy, 

Can you justify this change? I was just trying to fix a bug. It happens
to be fixed by this, but I was seeing it with already-existing resources
and wanted to fix it for those as well - plus I didn't see this change
until I'd found it and fixed it differently. This fix conflicts and
makes my fix fail (which isn't a problem in itself, since you've already
fixed it, but I don't think this makes sense).

I understand the neccesity to not just blindly dump out an arbitrary
displayname, but this seems the wrong place to put it. We need (for
webdav) to escape certain characters that have special meaning to xml. A
CDATA section is one way to do this. However, it's unneccesary (and
confusing) to put the actual XML content into the stores (which this,
indirectly, does). Instead, it'd make much more sense (to me, anyway) to
store the displayname in raw form in the stores (since we assume,
sensibly, that the stores can store arbitrary data), and then to
encapsulate this in a CDATA section on output as xml. 

This would have the additional benefit of fixing problems with existing
resources.

The fix then becomes removing this change, and changing a pair of
writeText() calls to writeData(), in PropFindMethod.java (the logical
place - the slide core shouldn't know about the output format of the
data, and doesn't in other places). If you agree, I'll commit this
change (just doing some testing on it now). 

Michael

Reply via email to