From 0da97e7a0352e8d4fd37fab2a7b2cb8c5b80f9ad Mon Sep 17 00:00:00 2001 From: "James D. Forrester" Date: Mon, 9 Apr 2018 09:56:03 -0700 Subject: [PATCH] Immediately drop wgValidateAllHtml and related code Bug: T191670 Change-Id: If13d02ee1b30fec1c701226af9d363c6e08b3737 --- RELEASE-NOTES-1.31 | 6 ++ includes/DefaultSettings.php | 6 -- includes/OutputHandler.php | 87 +---------------------------- includes/parser/MWTidy.php | 21 ------- includes/tidy/TidyDriverBase.php | 12 ---- tests/phpunit/MediaWikiTestCase.php | 55 ------------------ 6 files changed, 7 insertions(+), 180 deletions(-) diff --git a/RELEASE-NOTES-1.31 b/RELEASE-NOTES-1.31 index ebd978721b..f40a422adb 100644 --- a/RELEASE-NOTES-1.31 +++ b/RELEASE-NOTES-1.31 @@ -32,6 +32,7 @@ production. performance reasons, and installations with this setting will now work as if it was configured with 'any'. * $wgLogAutopatrol now defaults to false instead of true. +* $wgValidateAllHtml was removed and will be ignored. === New features in 1.31 === * (T76554) User sub-pages named ….json are now protected in the same way that ….js @@ -239,6 +240,11 @@ changes to languages because of Phabricator reports. * The ResourceLoaderGetLessVars hook, deprecated in 1.30, has been removed. Use ResourceLoaderModule::getLessVars() to expose local variables instead of global ones. +* As part of work to modernise user-generated content clean-up, a config option and some + methods related to HTML validity were removed without deprecation. The public methods + MWTidy::checkErrors() and its callee TidyDriverBase::validate() are removed, as are + MediaWikiTestCase::assertValidHtmlSnippet() and ::assertValidHtmlDocument(). The + $wgValidateAllHtml configuration option is removed and will be ignored. === Deprecations in 1.31 === * The Revision class was deprecated in favor of RevisionStore, BlobStore, and diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index c000098302..22f587ee15 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -3265,12 +3265,6 @@ $wgSiteNotice = ''; */ $wgSiteSupportPage = ''; -/** - * Validate the overall output using tidy and refuse - * to display the page if it's not valid. - */ -$wgValidateAllHtml = false; - /** * Default skin, for new users and anonymous visitors. Registered users may * change this to any one of the other available skins in their preferences. diff --git a/includes/OutputHandler.php b/includes/OutputHandler.php index 842922d566..16c37841c8 100644 --- a/includes/OutputHandler.php +++ b/includes/OutputHandler.php @@ -22,9 +22,6 @@ namespace MediaWiki; -use MWTidy; -use Html; - /** * @since 1.31 */ @@ -36,31 +33,10 @@ class OutputHandler { * @return string */ public static function handle( $s ) { - global $wgDisableOutputCompression, $wgValidateAllHtml, $wgMangleFlashPolicy; + global $wgDisableOutputCompression, $wgMangleFlashPolicy; if ( $wgMangleFlashPolicy ) { $s = self::mangleFlashPolicy( $s ); } - if ( $wgValidateAllHtml ) { - $headers = headers_list(); - $isHTML = false; - foreach ( $headers as $header ) { - $parts = explode( ':', $header, 2 ); - if ( count( $parts ) !== 2 ) { - continue; - } - $name = strtolower( trim( $parts[0] ) ); - $value = trim( $parts[1] ); - if ( $name == 'content-type' && ( strpos( $value, 'text/html' ) === 0 - || strpos( $value, 'application/xhtml+xml' ) === 0 ) - ) { - $isHTML = true; - break; - } - } - if ( $isHTML ) { - $s = self::validateAllHtml( $s ); - } - } if ( !$wgDisableOutputCompression && !ini_get( 'zlib.output_compression' ) ) { if ( !defined( 'MW_NO_OUTPUT_COMPRESSION' ) ) { $s = self::handleGzip( $s ); @@ -183,65 +159,4 @@ class OutputHandler { header( "Content-Length: $length" ); } } - - /** - * Replace the output with an error if the HTML is not valid. - * - * @param string $s - * @return string - */ - private static function validateAllHtml( $s ) { - $errors = ''; - if ( MWTidy::checkErrors( $s, $errors ) ) { - return $s; - } - - header( 'Cache-Control: no-cache' ); - - $out = Html::element( 'h1', null, 'HTML validation error' ); - $out .= Html::openElement( 'ul' ); - - $error = strtok( $errors, "\n" ); - $badLines = []; - while ( $error !== false ) { - if ( preg_match( '/^line (\d+)/', $error, $m ) ) { - $lineNum = intval( $m[1] ); - $badLines[$lineNum] = true; - $out .= Html::rawElement( 'li', null, - Html::element( 'a', [ 'href' => "#line-{$lineNum}" ], $error ) ) . "\n"; - } - $error = strtok( "\n" ); - } - - $out .= Html::closeElement( 'ul' ); - $out .= Html::element( 'pre', null, $errors ); - $out .= Html::openElement( 'ol' ) . "\n"; - $line = strtok( $s, "\n" ); - $i = 1; - while ( $line !== false ) { - $attrs = []; - if ( isset( $badLines[$i] ) ) { - $attrs['class'] = 'highlight'; - $attrs['id'] = "line-$i"; - } - $out .= Html::element( 'li', $attrs, $line ) . "\n"; - $line = strtok( "\n" ); - $i++; - } - $out .= Html::closeElement( 'ol' ); - - $style = << 'en', 'dir' => 'ltr' ] ) . - Html::rawElement( 'head', null, - Html::element( 'title', null, 'HTML validation error' ) . - Html::inlineStyle( $style ) ) . - Html::rawElement( 'body', null, $out ) . - Html::closeElement( 'html' ); - - return $out; - } } diff --git a/includes/parser/MWTidy.php b/includes/parser/MWTidy.php index 330859d4b5..19cf573157 100644 --- a/includes/parser/MWTidy.php +++ b/includes/parser/MWTidy.php @@ -52,27 +52,6 @@ class MWTidy { return $driver->tidy( $text ); } - /** - * Check HTML for errors, used if $wgValidateAllHtml = true. - * - * @param string $text - * @param string &$errorStr Return the error string - * @return bool Whether the HTML is valid - * @throws MWException - */ - public static function checkErrors( $text, &$errorStr = null ) { - $driver = self::singleton(); - if ( !$driver ) { - throw new MWException( __METHOD__ . - ': tidy is disabled, caller should have checked MWTidy::isEnabled()' ); - } - if ( $driver->supportsValidate() ) { - return $driver->validate( $text, $errorStr ); - } else { - throw new MWException( __METHOD__ . ": tidy driver does not support validate()" ); - } - } - /** * @return bool */ diff --git a/includes/tidy/TidyDriverBase.php b/includes/tidy/TidyDriverBase.php index f88b673479..a53360c9d9 100644 --- a/includes/tidy/TidyDriverBase.php +++ b/includes/tidy/TidyDriverBase.php @@ -20,18 +20,6 @@ abstract class TidyDriverBase { return false; } - /** - * Check HTML for errors, used if $wgValidateAllHtml = true. - * - * @param string $text - * @param string &$errorStr Return the error string - * @throws \MWException - * @return bool Whether the HTML is valid - */ - public function validate( $text, &$errorStr ) { - throw new \MWException( static::class . ' does not support validate()' ); - } - /** * Clean up HTML * diff --git a/tests/phpunit/MediaWikiTestCase.php b/tests/phpunit/MediaWikiTestCase.php index 4d424983c7..8b2d099227 100644 --- a/tests/phpunit/MediaWikiTestCase.php +++ b/tests/phpunit/MediaWikiTestCase.php @@ -1995,61 +1995,6 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase { return $loaded; } - /** - * Asserts that the given string is a valid HTML snippet. - * Wraps the given string in the required top level tags and - * then calls assertValidHtmlDocument(). - * The snippet is expected to be HTML 5. - * - * @since 1.23 - * - * @note Will mark the test as skipped if the "tidy" module is not installed. - * @note This ignores $wgUseTidy, so we can check for valid HTML even (and especially) - * when automatic tidying is disabled. - * - * @param string $html An HTML snippet (treated as the contents of the body tag). - */ - protected function assertValidHtmlSnippet( $html ) { - $html = 'test' . $html . ''; - $this->assertValidHtmlDocument( $html ); - } - - /** - * Asserts that the given string is valid HTML document. - * - * @since 1.23 - * - * @note Will mark the test as skipped if the "tidy" module is not installed. - * @note This ignores $wgUseTidy, so we can check for valid HTML even (and especially) - * when automatic tidying is disabled. - * - * @param string $html A complete HTML document - */ - protected function assertValidHtmlDocument( $html ) { - // Note: we only validate if the tidy PHP extension is available. - // In case wgTidyInternal is false, MWTidy would fall back to the command line version - // of tidy. In that case however, we can not reliably detect whether a failing validation - // is due to malformed HTML, or caused by tidy not being installed as a command line tool. - // That would cause all HTML assertions to fail on a system that has no tidy installed. - if ( !$GLOBALS['wgTidyInternal'] || !MWTidy::isEnabled() ) { - $this->markTestSkipped( 'Tidy extension not installed' ); - } - - $errorBuffer = ''; - MWTidy::checkErrors( $html, $errorBuffer ); - $allErrors = preg_split( '/[\r\n]+/', $errorBuffer ); - - // Filter Tidy warnings which aren't useful for us. - // Tidy eg. often cries about parameters missing which have actually - // been deprecated since HTML4, thus we should not care about them. - $errors = preg_grep( - '/^(.*Warning: (trimming empty|.* lacks ".*?" attribute).*|\s*)$/m', - $allErrors, PREG_GREP_INVERT - ); - - $this->assertEmpty( $errors, implode( "\n", $errors ) ); - } - /** * Used as a marker to prevent wfResetOutputBuffers from breaking PHPUnit. * @param string $buffer -- 2.20.1