From a3f63785eebfa2f729d42da880b6722736ef1e7d Mon Sep 17 00:00:00 2001 From: "C. Scott Ananian" Date: Fri, 26 Oct 2018 11:14:01 -0400 Subject: [PATCH] Deprecate OutputPage::parse() and OutputPage::parseInline() The OutputPage::parse() and OutputPage::parseInline() methods behave misleadingly different from the OutputPage::addWikitext*() methods: they don't tidy their output, they have different defaults for interface/content language selection, and they (sometimes) add wrapper divs. Deprecate these and add new methods with tidy output, clear language selection, and consistent defaults: OutputPage::parseAsContent(), OutputPage::parseAsInterface(), and OutputPage::parseInlineAsInterface(). Unify the implementation of the parse* methods with the addWikiText* methods, to reduce the likelihood that the behavior will diverge again in the future. Bug: T198214 Change-Id: Ica79c2acbc542ef37f971c0be2582ae771a23bd0 --- RELEASE-NOTES-1.33 | 5 ++ includes/OutputPage.php | 105 +++++++++++++++++----- tests/phpunit/includes/OutputPageTest.php | 104 ++++++++++++++++++++- 3 files changed, 189 insertions(+), 25 deletions(-) diff --git a/RELEASE-NOTES-1.33 b/RELEASE-NOTES-1.33 index 999cda8782..2b4951336c 100644 --- a/RELEASE-NOTES-1.33 +++ b/RELEASE-NOTES-1.33 @@ -78,6 +78,11 @@ because of Phabricator reports. applied for Arabic and Malayalam in the future. Please enable these on your local wiki (if you have them explicitly set to false) and run maintenance/cleanupTitles.php to fix any existing page titles. +* OutputPage::parse() and OutputPage::parseInline() have been deprecated + due to untidy output and inconsistent handling of wrapper divs and + interface/content language defaults. Use OutputPage::parseAsContent(), + OutputPage::parseAsInterface(), or OutputPage::parseInlineAsInterface() + as appropriate. * The LegacyHookPreAuthenticationProvider class, deprecated since its creation in 1.27 as part of the AuthManager re-write, now emits deprecation warnings. This will help identify the issue if you added it to $wgAuthManagerConfig. diff --git a/includes/OutputPage.php b/includes/OutputPage.php index 97d9d8333b..aa2afe962d 100644 --- a/includes/OutputPage.php +++ b/includes/OutputPage.php @@ -1935,23 +1935,14 @@ class OutputPage extends ContextSource { private function addWikiTextTitleInternal( $text, Title $title, $linestart, $tidy, $interface, $wrapperClass = null ) { - global $wgParser; - if ( !$tidy ) { wfDeprecated( 'disabling tidy', '1.32' ); } - $popts = $this->parserOptions(); - $oldTidy = $popts->setTidy( $tidy ); - $popts->setInterfaceMessage( (bool)$interface ); - - $parserOutput = $wgParser->getFreshParser()->parse( - $text, $title, $popts, - $linestart, true, $this->mRevisionId + $parserOutput = $this->parseInternal( + $text, $title, $linestart, $tidy, $interface, /*language*/null ); - $popts->setTidy( $oldTidy ); - $this->addParserOutput( $parserOutput, [ 'enableSectionEditLinks' => false, 'wrapperDivClass' => $wrapperClass ?? '', @@ -2102,15 +2093,79 @@ class OutputPage extends ContextSource { * @param Language|null $language Target language object, will override $interface * @throws MWException * @return string HTML + * @deprecated since 1.33, due to untidy output and inconsistent wrapper; + * use parseAsContent() if $interface is default value or false, or else + * parseAsInterface() if $interface is true. */ public function parse( $text, $linestart = true, $interface = false, $language = null ) { return $this->parseInternal( - $text, $linestart, $interface, $language + $text, $this->getTitle(), $linestart, /*tidy*/false, $interface, $language + )->getText( [ + 'enableSectionEditLinks' => false, + ] ); + } + + /** + * Parse wikitext *in the page content language* and return the HTML. + * The result will be language-converted to the user's preferred variant. + * Output will be tidy. + * + * @param string $text Wikitext in the page content language + * @param bool $linestart Is this the start of a line? (Defaults to true) + * @throws MWException + * @return string HTML + * @since 1.33 + */ + public function parseAsContent( $text, $linestart = true ) { + return $this->parseInternal( + $text, $this->getTitle(), $linestart, /*tidy*/true, /*interface*/false, /*language*/null )->getText( [ 'enableSectionEditLinks' => false, + 'wrapperDivClass' => '' ] ); } + /** + * Parse wikitext *in the user interface language* and return the HTML. + * The result will not be language-converted, as user interface messages + * are already localized into a specific variant. + * Output will be tidy. + * + * @param string $text Wikitext in the user interface language + * @param bool $linestart Is this the start of a line? (Defaults to true) + * @throws MWException + * @return string HTML + * @since 1.33 + */ + public function parseAsInterface( $text, $linestart = true ) { + return $this->parseInternal( + $text, $this->getTitle(), $linestart, /*tidy*/true, /*interface*/true, /*language*/null + )->getText( [ + 'enableSectionEditLinks' => false, + 'wrapperDivClass' => '' + ] ); + } + + /** + * Parse wikitext *in the user interface language*, strip + * paragraph wrapper, and return the HTML. + * The result will not be language-converted, as user interface messages + * are already localized into a specific variant. + * Output will be tidy. Outer paragraph wrapper will only be stripped + * if the result is a single paragraph. + * + * @param string $text Wikitext in the user interface language + * @param bool $linestart Is this the start of a line? (Defaults to true) + * @throws MWException + * @return string HTML + * @since 1.33 + */ + public function parseInlineAsInterface( $text, $linestart = true ) { + return Parser::stripOuterParagraph( + $this->parseAsInterface( $text, $linestart ) + ); + } + /** * Parse wikitext, strip paragraph wrapper, and return the HTML. * @@ -2119,10 +2174,14 @@ class OutputPage extends ContextSource { * @param bool $interface Use interface language (instead of content language) while parsing * language sensitive magic words like GRAMMAR and PLURAL * @return string HTML + * @deprecated since 1.33, due to untidy output and confusing default + * for $interface. Use parseInlineAsInterface() if $interface is + * the default value or false, or else use + * Parser::stripOuterParagraph($outputPage->parseAsContent(...)). */ public function parseInline( $text, $linestart = true, $interface = false ) { $parsed = $this->parseInternal( - $text, $linestart, $interface, /*language*/null + $text, $this->getTitle(), $linestart, /*tidy*/false, $interface, /*language*/null )->getText( [ 'enableSectionEditLinks' => false, 'wrapperDivClass' => '', /* no wrapper div */ @@ -2134,7 +2193,9 @@ class OutputPage extends ContextSource { * Parse wikitext and return the HTML (internal implementation helper) * * @param string $text + * @param Title The title to use * @param bool $linestart Is this the start of a line? + * @param bool $tidy Whether the output should be tidied * @param bool $interface Use interface language (instead of content language) while parsing * language sensitive magic words like GRAMMAR and PLURAL. This also disables * LanguageConverter. @@ -2142,29 +2203,29 @@ class OutputPage extends ContextSource { * @throws MWException * @return ParserOutput */ - private function parseInternal( $text, $linestart, $interface, $language ) { + private function parseInternal( $text, $title, $linestart, $tidy, $interface, $language ) { global $wgParser; - if ( is_null( $this->getTitle() ) ) { + if ( is_null( $title ) ) { throw new MWException( 'Empty $mTitle in ' . __METHOD__ ); } $popts = $this->parserOptions(); - if ( $interface ) { - $popts->setInterfaceMessage( true ); - } + $oldTidy = $popts->setTidy( $tidy ); + $oldInterface = $popts->setInterfaceMessage( (bool)$interface ); + if ( $language !== null ) { $oldLang = $popts->setTargetLanguage( $language ); } $parserOutput = $wgParser->getFreshParser()->parse( - $text, $this->getTitle(), $popts, + $text, $title, $popts, $linestart, true, $this->mRevisionId ); - if ( $interface ) { - $popts->setInterfaceMessage( false ); - } + $popts->setTidy( $oldTidy ); + $popts->setInterfaceMessage( $oldInterface ); + if ( $language !== null ) { $popts->setTargetLanguage( $oldLang ); } diff --git a/tests/phpunit/includes/OutputPageTest.php b/tests/phpunit/includes/OutputPageTest.php index cd20bb2c95..e572be215d 100644 --- a/tests/phpunit/includes/OutputPageTest.php +++ b/tests/phpunit/includes/OutputPageTest.php @@ -1878,24 +1878,122 @@ class OutputPageTest extends MediaWikiTestCase { ]; } + /** + * @dataProvider provideParseAs + * @covers OutputPage::parseAsContent + * @param array $args To pass to parse() + * @param string $expectedHTML Expected return value for parseAsContent() + * @param string $expectedHTML Expected return value for parseInlineAsInterface(), if different + */ + public function testParseAsContent( + array $args, $expectedHTML, $expectedHTMLInline = null + ) { + $op = $this->newInstance(); + $this->assertSame( $expectedHTML, $op->parseAsContent( ...$args ) ); + } + + /** + * @dataProvider provideParseAs + * @covers OutputPage::parseAsInterface + * @param array $args To pass to parse() + * @param string $expectedHTML Expected return value for parseAsInterface() + * @param string $expectedHTML Expected return value for parseInlineAsInterface(), if different + */ + public function testParseAsInterface( + array $args, $expectedHTML, $expectedHTMLInline = null + ) { + $op = $this->newInstance(); + $this->assertSame( $expectedHTML, $op->parseAsInterface( ...$args ) ); + } + + /** + * @dataProvider provideParseAs + * @covers OutputPage::parseInlineAsInterface + */ + public function testParseInlineAsInterface( + array $args, $expectedHTML, $expectedHTMLInline = null + ) { + $op = $this->newInstance(); + $this->assertSame( + $expectedHTMLInline ?? $expectedHTML, + $op->parseInlineAsInterface( ...$args ) + ); + } + + public function provideParseAs() { + return [ + 'List at start of line' => [ + [ '* List', true ], + "\n", + ], + 'List not at start' => [ + [ "* ''Not'' list", false ], + '

* Not list

', + '* Not list', + ], + 'Italics' => [ + [ "''Italic''", true ], + "

Italic\n

", + 'Italic', + ], + 'formatnum' => [ + [ '{{formatnum:123456.789}}', true ], + "

123,456.789\n

", + "123,456.789", + ], + 'No section edit links' => [ + [ '== Header ==' ], + '

Header

' . + "\n", + ] + ]; + } + /** * @covers OutputPage::parse */ public function testParseNullTitle() { - $this->setExpectedException( MWException::class, 'Empty $mTitle in OutputPage::parse' ); + $this->setExpectedException( MWException::class, 'Empty $mTitle in OutputPage::parseInternal' ); $op = $this->newInstance( [], null, 'notitle' ); $op->parse( '' ); } /** - * @covers OutputPage::parse + * @covers OutputPage::parseInline */ public function testParseInlineNullTitle() { - $this->setExpectedException( MWException::class, 'Empty $mTitle in OutputPage::parse' ); + $this->setExpectedException( MWException::class, 'Empty $mTitle in OutputPage::parseInternal' ); $op = $this->newInstance( [], null, 'notitle' ); $op->parseInline( '' ); } + /** + * @covers OutputPage::parseAsContent + */ + public function testParseAsContentNullTitle() { + $this->setExpectedException( MWException::class, 'Empty $mTitle in OutputPage::parseInternal' ); + $op = $this->newInstance( [], null, 'notitle' ); + $op->parseAsContent( '' ); + } + + /** + * @covers OutputPage::parseAsInterface + */ + public function testParseAsInterfaceNullTitle() { + $this->setExpectedException( MWException::class, 'Empty $mTitle in OutputPage::parseInternal' ); + $op = $this->newInstance( [], null, 'notitle' ); + $op->parseAsInterface( '' ); + } + + /** + * @covers OutputPage::parseInlineAsInterface + */ + public function testParseInlineAsInterfaceNullTitle() { + $this->setExpectedException( MWException::class, 'Empty $mTitle in OutputPage::parseInternal' ); + $op = $this->newInstance( [], null, 'notitle' ); + $op->parseInlineAsInterface( '' ); + } + /** * @covers OutputPage::setCdnMaxage * @covers OutputPage::lowerCdnMaxage -- 2.20.1