Page MenuHomePhabricator

Determine future of MWTidy::checkErrors in a Remex world
Closed, ResolvedPublic

Description

Currently RemexDriver does not implement validate(), so MWTidy:checkErrors() throws an exception. The function is used in MediaWikiTestCase::assertValidHtmlDocument() and OutputHandler (if $wgValidateAllHtml = true;).

Event Timeline

Legoktm created this task.Apr 6 2018, 9:48 PM

It isn't clear what "validate" is supposed to be doing. In the Tidy world, it seems to be checking that Tidy doesn't crash.

HTML5 spec has the notion of parse errors, but as expected, given that browsers encounter all kinds of html out in the wild, the html5 parser provides error recovery conditions for parse errors. Parse errors are only relevant for conformance checkers that might abort parsing on the first error, but is not a suitable response for browsers or wikitext parsers.

Looks like the capiunto extension uses this. As far as I can tell, the use in https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/Capiunto/+/master/tests/phpunit/output/BasicRowTest.php#42 isn't really testing anything interesting. It seems like defensive code. BasicRowTest.lua just creates a table with a header and a row.

So, given that the lone use case is basically useless, I support legoktm's proposal to get rid of this mode from testing.

hoo added a comment.Apr 9 2018, 4:40 PM

Capiunto alone certainly isn't enough to justify maintaining/ having this… can be ditched IMO.

Capiunto alone certainly isn't enough to justify maintaining/ having this… can be ditched IMO.

OK: Capiunto removal in https://gerrit.wikimedia.org/r/#/c/425091/ and MW core immediate removal (!) in https://gerrit.wikimedia.org/r/#/c/425093/

Krinkle added a subscriber: Krinkle.Apr 9 2018, 5:13 PM

+1 for removal of this feature. I previously suggested this at https://gerrit.wikimedia.org/r/420224 (4321128f15748) as part of T189966.

Change 425093 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/core@master] [WIP] Immediately drop wgValidateAllHtml and related code

https://gerrit.wikimedia.org/r/425093

Change 425093 merged by jenkins-bot:
[mediawiki/core@master] Immediately drop wgValidateAllHtml and related code

https://gerrit.wikimedia.org/r/425093

Jdforrester-WMF closed this task as Resolved.Apr 12 2018, 3:24 PM
Jdforrester-WMF claimed this task.
Jdforrester-WMF removed a project: Patch-For-Review.