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

Atsushi Eno atsushi at ximian.com
Mon Jun 27 05:44:03 EDT 2005


>> 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.

It's different from valid-sa-086. It tests *invalid* document
(that one I pointed out why your previous patch was not fine).

>> 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.

Oh, I see. There's another reason to fix entity resolution stuff
should be improved. Mhm, actually the testcase I've attached is
a strong counterpart of temptation to change implementation to
make it to take lazy evaluation way :(

>>
>> (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.

That's exactly the reason why you *don't* need "-AS" there ;-) (and
most of us won't know what AS means. At least "- Andrew" is much
better (but we usually don't record our names. Yeah, it depends on
the projects and organizations though).

>>
>> 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.

That's part of why I don't love that enormous tests. Maybe no one
including you will never fix those "big" problem, or make
significant contributions. I'd even like to accept some regressions
in front of such big improvements if any. XML standalone tests are
so canonical pretty enough.

Atsushi Eno



More information about the Mono-devel-list mailing list