Quoting Dries: "I'm wondering if we could add a XHTML validation step to D7's test framework so that every page that is tested automatically gets validated ..."
Cross posting from #326527: form id="node-form" declared twice

CommentFileSizeAuthor
#4 XHTML_validation.patch193.78 KBmr.baileys
#2 simpletest_dom.patch3.76 KBboombatower

Comments

boombatower’s picture

boombatower commented
Assigned: Unassigned » boombatower

Since we already use DOM to parse document should be easy.

DOMDocument::validate -> http://us3.php.net/manual/en/domdocument.validate.php

boombatower’s picture

boombatower commented
Status: Active » Needs review
StatusFileSize
new simpletest_dom.patch3.76 KB

Here is some testing I have done.

The validation seems to work great...checks some subtle things I tried.

I included some code MYDOMDocument in a user comment to collect the actually errors. This may/maynot be something we want to collect. Also the class requires cleanup (and I can't get to work either).

Status: Needs review » Needs work

The last submitted patch failed testing.

mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
commented
Issue tags: +Invalid markup, +XHTML, +XHTML validation
StatusFileSize
new XHTML_validation.patch193.78 KB

I had something similar over at #402356: XHTML validator needed in SimpleTest. (now marked as duplicate).

Some comments after reviewing your patch:

  1. As I understand it, the MyDOMDocument class is used to trap the errors thrown by the validation functions. I think using libxml_use_internal_errors() and libxml_get_errors() is cleaner and shorter (although I'm not sure about compatibility across PHP (sub-)versions).
  2. When I used DOMDocument::validate(), the performance was terrible. I traced this down to the DTD (and its related resources) being downloaded on each request. I've found that XML caches are the solution for this, but couldn't get one set up on my end (Windows...), and doing so automatically doesn't seem feasible. I'm wondering what your experience with regards to performance is? Maybe this is a Windows issue? My final solution was to bundle the DTDs with SimpleTest, but this results in a large patch (which ni turn might make it unsuitable to be commited in the end) and might prove more difficult to manage in the long term.

I've attached my patch here for comparison/documentation (I don't mean to hijack your issue). Also set this to CNR to let the testbot play with it once.

Also note that there is a similar effort by Damien Tournoud in (#402254: How do we do on XHTML validation?). While not full XHTML validation, it does bring to light a number of bugs by just changing one line in SimpleTest.

mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
commented
Status: Needs work » Needs review

Also set this to CNR to let the testbot play with it once.

boombatower’s picture

boombatower commented

The performance I got was terrible as well and that was the same conclusion I drew...and if you delete the DTD references from the template it runs better (my hack to confirm).

Thus I am open to whatever solution you have if it runs better. I just remembered that existed and played with it to see what I could get to work. Glad to see someone else interested in this.

mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
commented

Problem is: by dropping the DTD I think we're defeating the purpose of XHTML validation, since it's the DTD that drives most of it. Haven't tested this, but my fear is that be removing the DTD, we're essentially downgrading to just HTML validation.

Maybe we should start by building a test case containing things we want the validator to notice? (note that some of these will get caught even by normal HTML validation as they're not XHTML specific):

  • Uppercase element names
  • Unclosed tags
  • Unmatched tags
  • Disallowed tags
  • Tags that are only allowed within certain other tags
  • Invalid element attributes
  • Duplicate ID attributes
  • Incorrectly encoded entities
  • ...

Status: Needs review » Needs work

The last submitted patch failed testing.

boombatower’s picture

boombatower commented

I don't think we should remove the DTD tags...I would just commenting that that in fact seems to be the problem and we need some sort of solution to make this feasible.

dries’s picture

dries commented
Issue tags: +Favorite-of-Dries

Awesome. One option, but I'm not sure I like that, is to make the validation optional; i.e. there could be a checkbox on the SimpleTest page that allows people to enable/disable validation. HTML validation is probably not something we'd need to run all the time, if gets run once in a while, that might be "good enough".

damien tournoud’s picture

damien tournoud commented

I don't think we actually need all that, at least not right now. DOMDocument::loadHTML() seems to know a lot about HTML, including (at least part of) the DTDs. I just posted a simple patch to suppress error hiding in #402254: How do we do on XHTML validation?... it resulted in 1442 exceptions. We should first concentrate on fixing those, then come back to full-fledged validation to see if it makes a difference.

damien tournoud’s picture

damien tournoud commented
Status: Needs work » Postponed

Let's postpone this one until we investigate the 1442 exceptions of #402254: How do we do on XHTML validation?.

damien tournoud’s picture

damien tournoud commented
Issue tags: -Invalid markup, -XHTML

Uniformizing tags.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

+1 for this. Would be even better if we could somehow get a way to validate the RSS/XML feeds that Drupal generates.

ohnobinki’s picture

ohnobinki commented

+1

mgifford’s picture

mgifford
he/him
Primary language English
commented
Version: 7.x-dev » 8.x-dev

What happened to drupal_web_test_case.php anyways?

Other than that (and the move within the core/ directory) the patch applies pretty nicely still.

Any thoughts on addressing this in D8?

EDIT: Just occurred to me that maybe this was still at D7 because it is all xhtml not HTML5.

Still occurs to me that we should have this validation.

hanno’s picture

hanno commented

Yes, a validation check is also a good idea for HTML5. In a DTAP development process it's great when developers get feedback about the HTML they produce.

mgifford’s picture

mgifford
he/him
Primary language English
commented
Title: Add XHTML validation to simpletest » Add HTML validation to simpletest
Issue tags: +html5, +HTML Validation

I dislike acronyms - http://en.wikipedia.org/wiki/Development,_testing,_acceptance_and_produc...

But yes, absolutely still an issue in HTML5, so generalizing the title and tagging.

hanno’s picture

hanno commented

HTML5 validation is not yet available in Drupal #1333730: [Meta] PHP DOM (libxml2) misinterprets HTML5
Postpone this issue until we have a validation function to use?

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Title: Add HTML validation to simpletest » Add HTML validation to functional tests
Issue summary: View changes
Issue tags: +mink

We're not updating WTB or simpletest any more, but an option for automatic HTML validation might be a good thing to add to our Mink driver.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Postponed » Active

Also we shouldn't be postponed on #402254: How do we do on XHTML validation? which is from 2014.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

donquixote’s picture

donquixote commented

This issue is now focused on D8.

For D7 we can do this simple change first: #3191922: Collect DOMDocument errors in web test, instead of suppressing them.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.