Invalid XHTML-1.0-Strict rendering

[Remy Blank <remy.blank@…>]

Various pages don't pass XHTML-1.0-Strict validation in the 0.11-stable branch:

• All pages with an empty ctxtnav: empty <ul>
• Wiki history, diff: <input> at wrong location
• Tickets: <form> with name attribute
• Milestone: empty <table>, <noscript> and <input> at wrong location
• Milestone edit: <input> at wrong location
• Admin: missing action attribute in <form>, <form> with method="POST" instead of post
• About: usage of <tr> in a JavaScript string outside of a CDATA section

I will try to provide patches for all these issues.

[Remy Blank <remy.blank@…>]

Patch against 0.11-stable [7352] fixing several XHTML-1.0-Strict validation errors

[Remy Blank <remy.blank@…>]

A few explanations about the patch above:

• <form> tags must have an action attribute.
• The content of the method attribute of a <form> must be lowercase.
• JavaScript code containing < or & must be enclosed in a CDATA section or have these characters escaped.
• <input>, <label> and <select> tags must be in a block.
• Empty <ul> or <table> elements are not allowed.
• The content of a <noscript> tag must be a block level element in XHTML-1.0-Strict. As this was the only occurrence of such a tag in Trac, I fixed it by using JavaScript to hide the button instead of enclosing it in a <noscript> tag.
• <form> tags can only have an id attribute and must not have a name attribute. This required changing some functional tests, as one form had both with different names.

With the patch applied, all unit and functional tests pass (except for ticket.tests.functional.TestAdminMilestoneCompletedFuture, but only between 00:00 and 02:00 at night. No, I'm not kidding. I'm pretty sure this is a bug in the test itself, more precisely in the calculation of the completion date in the future. My timezone happens to be GMT+02:00, and I suspect this is related. I would suggest using a completion time several days in the future).

[cboos]

Testing, looks good so far.

[osimons]

Replying to Remy Blank <remy.blank@pobox.com>:

With the patch applied, all unit and functional tests pass (except for ticket.tests.functional.TestAdminMilestoneCompletedFuture, but only between 00:00 and 02:00 at night. No, I'm not kidding. I'm pretty sure this is a bug in the test itself, more precisely in the calculation of the completion date in the future. My timezone happens to be GMT+02:00, and I suspect this is related. I would suggest using a completion time several days in the future).

Hehe. At 00:30 CET I ran into that test failure as well. Fixed in [7370:7371], and decided to stick with 1 day and just account for local timezone. Always useful to have borderline-calculations in unit tests...

Carry on with the actual topic of the ticket, please :-)

[cboos]

Replying to Remy Blank <remy.blank@pobox.com>:

* <form> tags can only have an id attribute and must not have a name attribute. This required changing some functional tests, as one form had both with different names.

Last minute hesitation: was the change regarding the id attribute really needed? i.e. can't we just drop the name attribute and keep the id as it is? (some people may have used #propertyform in their custom style sheets).

[Remy Blank <remy.blank@…>]

Replying to cboos:

Last minute hesitation: was the change regarding the id attribute really needed? i.e. can't we just drop the name attribute and keep the id as it is? (some people may have used #propertyform in their custom style sheets).

Well, I had to select one of the two names. Both are referenced in the functional tests, and there were more propform references than propertyform, so I selected the former.

But your argument is valid. Give me a few minutes and I'll drop the name="propform" attribute and leave id unchanged instead.

[Remy Blank <remy.blank@…>]

Same patch, but with ticket form id set to propertyform instead of propform

[Remy Blank <remy.blank@…>]

The second patch keeps the propertyform id and just removes the propform name. I have also rebased the patch to [7384]. All tests pass here.

[cboos]

Second patch committed in r7388. Thanks!

[Remy Blank <remy.blank@…>]

I found a few more, one empty <ul>, a few empty <tbody> tags in special cases (empty query result list, empty file list, empty revision list), and an empty <optgroup> (milestone selection in tickets).

[Remy Blank <remy.blank@…>]

Patch against 0.11-stable fixing XHTML validation errors

[Remy Blank <remy.blank@…>]

The empty <tbody> tags occurred in the following cases:

• An empty ticket query result list
• An empty file list
• An empty revision list

The two latter typically occur when the Subversion repository is empty. In each case, I added a single line saying "No {items} found" with {items} = tickets, files, revisions. I find this also looks better compared to an empty table.

[Remy Blank <remy.blank@…>]

Patch against 0.11-stable adding XHTML validation to all functional tests

[Remy Blank <remy.blank@…>]

The patch above is how I found these validation errors. It adds XHTML validation to all functional tests by hooking into Twill and validating every parsed page. Validation is done using PyXML and is optional, i.e. it is skipped if PyXML is not installed.

The systematic validation adds about 15% to the execution time of the complete test suite. The size of the patch is due to its containing the XHTML DTD files.

I would have added Eli Carter to Cc, as he seems to be the most active tester, but it seems that I don't have the permissions to do that.

[Remy Blank <remy.blank@…>]

Patch against 0.11-stable adding XHTML validation to all functional tests (fixed normpath -> abspath)

[cboos]

Looks like PyXML is not supported anymore (that's what the PyXML download page on sourceforge says). Wouldn't it be better to use lxml for validation?

[Remy Blank <remy.blank@…>]

Argh... Before starting, I researched what validating XML parsers were available in Python, and I narrowed the choice down to PyXML and lxml. I had both installed on my machine, and lxml seemed to require an additional library (libxml2), so I went with the former. I hadn't seen the note on the SourceForge download page.

Looking some more, I see that PyXML actually also requires an additional library (expat), so this point is moot anyway.

Let me try with lxml. It looks very interesting, so this will be a good exercise.

[Remy Blank <remy.blank@…>]

Patch against 0.11-stable adding XHTML validation to all functional tests using lxml

[Remy Blank <remy.blank@…>]

The patch above validates with lxml instead of PyXML. The lxml documentation is a bit obscure, and it took me a while to figure out how to load the DTD from disk instead of from the network.

The patch also contains the fixes for the remaining validation errors.

[anonymous]

Replying to Remy Blank <remy.blank@…>:

Argh... Before starting, I researched what validating XML parsers were available in Python, and I narrowed the choice down to PyXML and lxml. I had both installed on my machine, and lxml seemed to require an additional library (libxml2), so I went with the former. I hadn't seen the note on the SourceForge download page.

PyXML development has stalled so it is not a viable option.

[jonas]

I'm afraid I can't seem to get this patch to work right now. For some reason the custom lxml Resolver does not kick in so lxml tries to download xhtml1-strict.dtd from w3.org instead of using the bundled copy. I'm using lxml(2.1.1), libxml2(2.6.32) and libxslt(1.1.24) on osx leopard.

So I'm afraid this patch will have to wait until 0.11.2 since I'm hoping to release 0.11.1 really soon now.

[Remy Blank <remy.blank@…>]

Replying to jonas:

I'm using lxml(2.1.1), libxml2(2.6.32) and libxslt(1.1.24) on osx leopard.

I'm using lxml-2.0.5, libxml2-2.6.31 and libxslt-1.1.24 on Gentoo. Most probably something has changed in lxml. I'll see if I can package lxml-2.1.1 for Gentoo (it's not available yet) and try to reproduce the problem.

So I'm afraid this patch will have to wait until 0.11.2 since I'm hoping to release 0.11.1 really soon now.

It's low priority anyway, so this is fine. You could have pulled only the changes that fix validation errors from the patch, but this can certainly wait for 0.11.2 as well.

Thanks for trying the patch!

[Remy Blank <remy.blank@…>]

Strange. I have upgraded to lxml-2.1.1 and libxml2-2.6.32, and the patch still works well here. Maybe this is OSX-specific? I unfortunately don't have access to an OSX machine.

You may want to print the values of system_url, public_id, the first parameter passed to self.resolve_filename() and its result in better_twill._Resolver.resolve() to get an idea of what's going wrong.

I'll try to get access to an OSX Leopard machine somehow.

[osimons]

Noticed the added note in 4:ticket:7503:

When the patch in #7445 will be applied, the table will have a single line containing "No files found" for an empty repository.

This has quite a few similarities with #5549, although that concerns just a message for diff rendering: 'no files' can occur for a number of reasons... I see this patch adds a number of 'no <something>' content, and we need to make sure that the message makes sense for all/most common reasons (typically permissions).

[jonas]

I finally managed to track down the problem I was seeing. For some reason lxml didn't work if it was imported after twill.

After a lot of digging I found that the mac specific module "ic" (used by urllib.getproxies) breaks lxml for some unknown reason.

In [7458] I applied a modified version of the patch which should work on OSX as well.