Deprecate OutputPage::parse() and OutputPage::parseInline()
authorC. Scott Ananian <cscott@cscott.net>
Fri, 26 Oct 2018 15:14:01 +0000 (11:14 -0400)
committerC. Scott Ananian <cscott@cscott.net>
Mon, 29 Oct 2018 19:34:40 +0000 (15:34 -0400)
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
includes/OutputPage.php
tests/phpunit/includes/OutputPageTest.php

index 999cda8..2b49513 100644 (file)
@@ -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.
index 97d9d83..aa2afe9 100644 (file)
@@ -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 );
                }
index cd20bb2..e572be2 100644 (file)
@@ -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 ],
+                               "<ul><li>List</li></ul>\n",
+                       ],
+                       'List not at start' => [
+                               [ "* ''Not'' list", false ],
+                               '<p>* <i>Not</i> list</p>',
+                               '* <i>Not</i> list',
+                       ],
+                       'Italics' => [
+                               [ "''Italic''", true ],
+                               "<p><i>Italic</i>\n</p>",
+                               '<i>Italic</i>',
+                       ],
+                       'formatnum' => [
+                               [ '{{formatnum:123456.789}}', true ],
+                               "<p>123,456.789\n</p>",
+                               "123,456.789",
+                       ],
+                       'No section edit links' => [
+                               [ '== Header ==' ],
+                               '<h2><span class="mw-headline" id="Header">Header</span></h2>' .
+                                       "\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