[Fwd: [Mono-devel-list] Re: System.Xml patch]

[Andrew Skiba]

Atsushi Eno wrote:
> Please go ahead as I wrote before ;-)
Ugh, I said you I miss sometimes messages from devlist. Sorry for 
inconvenience.

> This patch looks fine. Let's check it in svn.
> 
> I'll apply the patch attached after your commit.

Actually, you don't have to add this nunit test (I am talking about 
EntityDeclarationNotWF ). It already exists in W3C, it's id is valid-sa-086.

> What does this mean? I guess you have in mind that there are cases that
> entities could be used inside attributes, but attribute content check
> is less prohibiting and '<' characters are checked anyways.

Not only in attributes. One of the tests went through the following flow:
DTD had entity declaration pointing to external xml, which had in turn 
it's own DTD. It is in valid-sa-100 test. So what happened is because 
current implementation makes early evaluation, it read the external 
source and failed because the context was Element, and Element can't 
have DTDs. So here we have a coincident of 2 things: early evaluation 
and Element context. So I understood that it's not always might be in 
Element context.

> 
> (BTW we won't understand what "-AS" means there ;-)

svn blame will help curious hackers ;-) but if it's not acceptable, I'll 
remove it.

> 
> Besides the comment itself.
> 
> In fact I also don't like that part of code. Actually, though
> replacing entities with a simple string is good for performance
> as compared to such code that loads entity possibly from external
> files every time, it causes incorrect BaseURI resolution (that
> results in incorrect not-wf error for XHTML 1.1 DTD; bug #51495).
> 
> So, basically rewriting that entity expansion part is the best
> solution. But I haven't tried that since it will not be done as a
> quick hack and it will first result in several breakage in
> standalone tests.

That's why I made this patch very carefully, to make minimal impact. I 
don't have plans to improve this code right now.

Cheers,
Andrew.