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

diff --git a/RELEASE-NOTES-1.33 b/RELEASE-NOTES-1.33
index 999cda8..2b49513 100644 (file)
--- 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.
   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.
 * 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 97d9d83..aa2afe9 100644 (file)
--- 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
        ) {
        private function addWikiTextTitleInternal(
                $text, Title $title, $linestart, $tidy, $interface, $wrapperClass = null
        ) {

-               global $wgParser;
-

                if ( !$tidy ) {
                        wfDeprecated( 'disabling tidy', '1.32' );
                }
 
                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 ?? '',
                $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
         * @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(
         */
        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,
                )->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.
         *
        /**
         * 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
         * @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(
         */
        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 */
                )->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
         * 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 $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.
         * @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
         */
         * @throws MWException
         * @return ParserOutput
         */

-       private function parseInternal( $text, $linestart, $interface, $language ) {
+       private function parseInternal( $text, $title, $linestart, $tidy, $interface, $language ) {

                global $wgParser;
 
                global $wgParser;
 

-               if ( is_null( $this->getTitle() ) ) {
+               if ( is_null( $title ) ) {

                        throw new MWException( 'Empty $mTitle in ' . __METHOD__ );
                }
 
                $popts = $this->parserOptions();
                        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(
                if ( $language !== null ) {
                        $oldLang = $popts->setTargetLanguage( $language );
                }
 
                $parserOutput = $wgParser->getFreshParser()->parse(

-                       $text, $this->getTitle(), $popts,
+                       $text, $title, $popts,

                        $linestart, true, $this->mRevisionId
                );
 
                        $linestart, true, $this->mRevisionId
                );
 

-               if ( $interface ) {
-                       $popts->setInterfaceMessage( false );
-               }
+               $popts->setTidy( $oldTidy );
+               $popts->setInterfaceMessage( $oldInterface );
+

                if ( $language !== null ) {
                        $popts->setTargetLanguage( $oldLang );
                }
                if ( $language !== null ) {
                        $popts->setTargetLanguage( $oldLang );
                }

diff --git a/tests/phpunit/includes/OutputPageTest.php b/tests/phpunit/includes/OutputPageTest.php
index cd20bb2..e572be2 100644 (file)
--- 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 ],
+                               "<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() {
        /**
         * @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( '' );
        }
 
        /**
                $op = $this->newInstance( [], null, 'notitle' );
                $op->parse( '' );
        }
 
        /**

-        * @covers OutputPage::parse
+        * @covers OutputPage::parseInline

         */
        public function testParseInlineNullTitle() {
         */
        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( '' );
        }
 
                $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
        /**
         * @covers OutputPage::setCdnMaxage
         * @covers OutputPage::lowerCdnMaxage