Deprecate and rename OutputPage::addWikiText* methods

Tidy will always be enabled with our future parsers, and it is fast
and pure PHP now with the Remex implementation, so deprecate all the
untidy variants of 'OutputPage::addWikiText*()' and add new methods
which tidy by default.  Clarify the content language/interface
language distinction while we're at it by adding 'AsInterface' to the
name of methods which use the "interface language" by default,
and renaming the 'addWikiText*Tidy' methods to
'addWikiTextAsContent'.

The 'OutputPage::addWikiTextTitle' method has been deprecated, but it
is still used internally as the implementation for the newly-added
methods.  It is expected that the shared implementation will move in
the future to a new private method.  Setting the `$tidy` parameter of
`OutputPage::addWikiTextTitle` to false is independently deprecated;
for backwards-compatibility with old MW releases you may wish to
continue to invoke OutputPage::addWikiTextTitle() but set $tidy=true;
this will result in the same tidied output that the newly added
methods would produce.

Bug: T198214
Change-Id: Ia58910164baaca608cea3b24333b7d13ed773339

diff --git a/RELEASE-NOTES-1.32 b/RELEASE-NOTES-1.32
index ec34acb..d929453 100644 (file)
--- a/RELEASE-NOTES-1.32
+++ b/RELEASE-NOTES-1.32
@@ -473,6 +473,14 @@ because of Phabricator reports.
   $wgTidyConfig instead.
 * All Tidy configurations other than Remex have been hard deprecated;
   future parsers will not emit compatible output for these configurations.
+* (T198214) OutputPage::addWikiText(), OutputPage::addWikiTextWithTitle(),
+  and OutputPage::addWikiTextTitle() have been deprecated, since they
+  can result in untidy output.  In addition OutputPage::addWikiTextTidy()
+  and OutputPage::addWikiTextTitleTidy() was deprecated to make naming new
+  methods consistent.  Use OutputPage::addWikiTextAsInterface() or
+  OutputPage::addWikiTextAsContent() instead, which ensures the output is
+  tidy and clarifies whether content-language specific postprocessing should
+  be done on the text.
 * QuickTemplate::msgHtml() and BaseTemplate::msgHtml() have been deprecated
   as they promote bad practises. I18n messages should always be properly
   escaped.

diff --git a/includes/OutputPage.php b/includes/OutputPage.php
index dd2f5ac..5801a29 100644 (file)
--- a/includes/OutputPage.php
+++ b/includes/OutputPage.php
@@ -1748,45 +1748,113 @@ class OutputPage extends ContextSource {
         * @param bool $linestart Is this the start of a line?
         * @param bool $interface Is this text in the user interface language?
         * @throws MWException
+        * @deprecated since 1.32 due to untidy output; use
+        *    addWikiTextAsInterface() if $interface is default value or true,
+        *    or else addWikiTextAsContent() if $interface is false.
         */
        public function addWikiText( $text, $linestart = true, $interface = true ) {
-               $title = $this->getTitle(); // Work around E_STRICT
+               $title = $this->getTitle();
                if ( !$title ) {
                        throw new MWException( 'Title is null' );
                }
                $this->addWikiTextTitle( $text, $title, $linestart, /*tidy*/false, $interface );
        }
 
+       /**
+        * Convert wikitext *in the user interface language* to HTML and
+        * add it to the buffer. The result will not be
+        * language-converted, as user interface messages are already
+        * localized into a specific variant.  Assumes that the current
+        * page title will be used if optional $title is not
+        * provided. 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)
+        * @param Title|null $title Optional title to use; default of `null`
+        *   means use current page title.
+        * @throws MWException if $title is not provided and OutputPage::getTitle()
+        *   is null
+        * @since 1.32
+        */
+       public function addWikiTextAsInterface(
+               $text, $linestart = true, Title $title = null
+       ) {
+               if ( $title === null ) {
+                       $title = $this->getTitle();
+               }
+               if ( !$title ) {
+                       throw new MWException( 'Title is null' );
+               }
+               $this->addWikiTextTitle( $text, $title, $linestart, /*tidy*/true, /*interface*/true );
+       }
+
+       /**
+        * Convert wikitext *in the page content language* to HTML and add
+        * it to the buffer.  The result with be language-converted to the
+        * user's preferred variant.  Assumes that the current page title
+        * will be used if optional $title is not provided. 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)
+        * @param Title|null $title Optional title to use; default of `null`
+        *   means use current page title.
+        * @throws MWException if $title is not provided and OutputPage::getTitle()
+        *   is null
+        * @since 1.32
+        */
+       public function addWikiTextAsContent(
+               $text, $linestart = true, Title $title = null
+       ) {
+               if ( $title === null ) {
+                       $title = $this->getTitle();
+               }
+               if ( !$title ) {
+                       throw new MWException( 'Title is null' );
+               }
+               $this->addWikiTextTitle( $text, $title, $linestart, /*tidy*/true, /*interface*/false );
+       }
+
        /**
         * Add wikitext with a custom Title object
         *
         * @param string $text Wikitext
         * @param Title $title
         * @param bool $linestart Is this the start of a line?
+        * @deprecated since 1.32 due to untidy output; use
+        *   addWikiTextAsInterface()
         */
        public function addWikiTextWithTitle( $text, Title $title, $linestart = true ) {
                $this->addWikiTextTitle( $text, $title, $linestart );
        }
 
        /**
-        * Add wikitext with a custom Title object and tidy enabled.
+        * Add wikitext *in content language* with a custom Title object.
+        * Output will be tidy.
         *
-        * @param string $text Wikitext
+        * @param string $text Wikitext in content language
         * @param Title $title
         * @param bool $linestart Is this the start of a line?
+        * @deprecated since 1.32 to rename methods consistently; use
+        *   addWikiTextAsContent()
         */
        function addWikiTextTitleTidy( $text, Title $title, $linestart = true ) {
                $this->addWikiTextTitle( $text, $title, $linestart, true );
        }
 
        /**
-        * Add wikitext with tidy enabled
+        * Add wikitext *in content language*. Output will be tidy.
         *
-        * @param string $text Wikitext
+        * @param string $text Wikitext in content language
         * @param bool $linestart Is this the start of a line?
+        * @deprecated since 1.32 to rename methods consistently; use
+        *   addWikiTextAsContent()
         */
        public function addWikiTextTidy( $text, $linestart = true ) {
                $title = $this->getTitle();
+               if ( !$title ) {
+                       throw new MWException( 'Title is null' );
+               }
                $this->addWikiTextTitleTidy( $text, $title, $linestart );
        }
 
@@ -1797,9 +1865,17 @@ class OutputPage extends ContextSource {
         * @param string $text Wikitext
         * @param Title $title
         * @param bool $linestart Is this the start of a line?
-        * @param bool $tidy Whether to use tidy
+        * @param bool $tidy Whether to use tidy.
+        *             Setting this to false (or omitting it) is deprecated
+        *             since 1.32; all wikitext should be tidied.
+        *             For backwards-compatibility with prior MW releases,
+        *             you may wish to invoke this method but set $tidy=true;
+        *             this will result in equivalent output to the non-deprecated
+        *             addWikiTextAsContent()/addWikiTextAsInterface() methods.
         * @param bool $interface Whether it is an interface message
         *   (for example disables conversion)
+        * @deprecated since 1.32, use addWikiTextAsContent() or
+        *   addWikiTextAsInterface() (depending on $interface)
         */
        public function addWikiTextTitle( $text, Title $title, $linestart,
                $tidy = false, $interface = false

diff --git a/tests/phpunit/includes/OutputPageTest.php b/tests/phpunit/includes/OutputPageTest.php
index 054636e..f9c293c 100644 (file)
--- a/tests/phpunit/includes/OutputPageTest.php
+++ b/tests/phpunit/includes/OutputPageTest.php
@@ -1392,6 +1392,8 @@ class OutputPageTest extends MediaWikiTestCase {
        /**
         * @dataProvider provideAddWikiText
         * @covers OutputPage::addWikiText
+        * @covers OutputPage::addWikiTextAsInterface
+        * @covers OutputPage::addWikiTextAsContent
         * @covers OutputPage::addWikiTextWithTitle
         * @covers OutputPage::addWikiTextTitle
         * @covers OutputPage::addWikiTextTidy
@@ -1409,6 +1411,13 @@ class OutputPageTest extends MediaWikiTestCase {
                        // Special placeholder because we can't get the actual title in the provider
                        $args[1] = $op->getTitle();
                }
+               if ( in_array(
+                       $method,
+                       [ 'addWikiTextAsInterface', 'addWikiTextAsContent' ]
+               ) && count( $args ) >= 3 && $args[2] === null ) {
+                       // Special placeholder because we can't get the actual title in the provider
+                       $args[2] = $op->getTitle();
+               }
 
                $op->$method( ...$args );
                $this->assertSame( $expected, $op->getHTML() );
@@ -1417,6 +1426,7 @@ class OutputPageTest extends MediaWikiTestCase {
        public function provideAddWikiText() {
                $tests = [
                        'addWikiText' => [
+                               // Not tidied; this API is deprecated.
                                'Simple wikitext' => [
                                        [ "'''Bold'''" ],
                                        "<p><b>Bold</b>\n</p>",
@@ -1435,6 +1445,7 @@ class OutputPageTest extends MediaWikiTestCase {
                                ],
                        ],
                        'addWikiTextWithTitle' => [
+                               // Untidied; this API is deprecated
                                'With title at start' => [
                                        [ '* {{PAGENAME}}', Title::newFromText( 'Talk:Some page' ) ],
                                        "<ul><li>Some page</li></ul>\n",
@@ -1443,7 +1454,36 @@ class OutputPageTest extends MediaWikiTestCase {
                                        "* Some page",
                                ],
                        ],
-                       'addWikiTextTidy' => [
+                       'addWikiTextAsInterface' => [
+                               // Preferred interface: output is tidied
+                               'Simple wikitext' => [
+                                       [ "'''Bold'''" ],
+                                       "<p><b>Bold</b>\n</p>",
+                               ], 'Untidy wikitext' => [
+                                       [ "<b>Bold" ],
+                                       "<p><b>Bold\n</b></p>",
+                               ], 'List at start' => [
+                                       [ '* List' ],
+                                       "<ul><li>List</li></ul>\n",
+                               ], 'List not at start' => [
+                                       [ '* Not a list', false ],
+                                       '<p>* Not a list</p>',
+                               ], 'No section edit links' => [
+                                       [ '== Title ==' ],
+                                       "<h2><span class=\"mw-headline\" id=\"Title\">Title</span></h2>\n",
+                               ], 'With title at start' => [
+                                       [ '* {{PAGENAME}}', true, Title::newFromText( 'Talk:Some page' ) ],
+                                       "<ul><li>Some page</li></ul>\n",
+                               ], 'With title at start' => [
+                                       [ '* {{PAGENAME}}', false, Title::newFromText( 'Talk:Some page' ), false ],
+                                       "<p>* Some page</p>",
+                               ], 'Untidy input' => [
+                                       [ '<b>{{PAGENAME}}', true, Title::newFromText( 'Talk:Some page' ) ],
+                                       "<p><b>Some page\n</b></p>",
+                               ],
+                       ],
+                       'addWikiTextAsContent' => [
+                               // Preferred interface: output is tidied
                                'SpecialNewimages' => [
                                        [ "<p lang='en' dir='ltr'>\nMy message" ],
                                        '<p lang="en" dir="ltr">' . "\nMy message\n</p>"
@@ -1453,17 +1493,14 @@ class OutputPageTest extends MediaWikiTestCase {
                                ], 'List not at start' => [
                                        [ '* <b>Not a list', false ],
                                        '<p>* <b>Not a list</b></p>',
-                               ],
-                       ],
-                       'addWikiTextTitleTidy' => [
-                               'With title at start' => [
-                                       [ '* {{PAGENAME}}', Title::newFromText( 'Talk:Some page' ) ],
+                               ], 'With title at start' => [
+                                       [ '* {{PAGENAME}}', true, Title::newFromText( 'Talk:Some page' ) ],
                                        "<ul><li>Some page</li></ul>\n",
                                ], 'With title at start' => [
-                                       [ '* {{PAGENAME}}', Title::newFromText( 'Talk:Some page' ), false ],
+                                       [ '* {{PAGENAME}}', false, Title::newFromText( 'Talk:Some page' ), false ],
                                        "<p>* Some page</p>",
                                ], 'EditPage' => [
-                                       [ "<div class='mw-editintro'>{{PAGENAME}}", Title::newFromText( 'Talk:Some page' ) ],
+                                       [ "<div class='mw-editintro'>{{PAGENAME}}", true, Title::newFromText( 'Talk:Some page' ) ],
                                        '<div class="mw-editintro">' . "Some page\n</div>"
                                ],
                        ],
@@ -1480,16 +1517,29 @@ class OutputPageTest extends MediaWikiTestCase {
                        $tests['addWikiTextTitle']["$key (addWikiTextTitle)"] =
                                array_merge( [ $args ], array_slice( $val, 1 ) );
                }
-               foreach ( $tests['addWikiTextTidy'] as $key => $val ) {
-                       $args = [ $val[0][0], null, $val[0][1] ?? true, true, false ];
+               foreach ( $tests['addWikiTextAsInterface'] as $key => $val ) {
+                       $args = [ $val[0][0], $val[0][2] ?? null, $val[0][1] ?? true, true, true ];
                        $tests['addWikiTextTitle']["$key (addWikiTextTitle)"] =
                                array_merge( [ $args ], array_slice( $val, 1 ) );
                }
-               foreach ( $tests['addWikiTextTitleTidy'] as $key => $val ) {
-                       $args = [ $val[0][0], $val[0][1], $val[0][2] ?? true, true, false ];
+               foreach ( $tests['addWikiTextAsContent'] as $key => $val ) {
+                       $args = [ $val[0][0], $val[0][2] ?? null, $val[0][1] ?? true, true, false ];
                        $tests['addWikiTextTitle']["$key (addWikiTextTitle)"] =
                                array_merge( [ $args ], array_slice( $val, 1 ) );
                }
+               // addWikiTextTidy / addWikiTextTitleTidy were old aliases of
+               // addWikiTextAsContent
+               foreach ( $tests['addWikiTextAsContent'] as $key => $val ) {
+                       if ( count( $val[0] ) > 2 ) {
+                               $args = [ $val[0][0], $val[0][2], $val[0][1] ?? true ];
+                               $tests['addWikiTextTitleTidy']["$key (addWikiTextTitleTidy)"] =
+                                       array_merge( [ $args ], array_slice( $val, 1 ) );
+                       } else {
+                               $args = [ $val[0][0], $val[0][1] ?? true ];
+                               $tests['addWikiTextTidy']["$key (addWikiTextTidy)"] =
+                                       array_merge( [ $args ], array_slice( $val, 1 ) );
+                       }
+               }
 
                // We have to reformat our array to match what PHPUnit wants
                $ret = [];
@@ -1513,6 +1563,26 @@ class OutputPageTest extends MediaWikiTestCase {
                $op->addWikiText( 'a' );
        }
 
+       /**
+        * @covers OutputPage::addWikiTextAsInterface
+        */
+       public function testAddWikiTextAsInterfaceNoTitle() {
+               $this->setExpectedException( MWException::class, 'Title is null' );
+
+               $op = $this->newInstance( [], null, 'notitle' );
+               $op->addWikiTextAsInterface( 'a' );
+       }
+
+       /**
+        * @covers OutputPage::addWikiTextAsContent
+        */
+       public function testAddWikiTextAsContentNoTitle() {
+               $this->setExpectedException( MWException::class, 'Title is null' );
+
+               $op = $this->newInstance( [], null, 'notitle' );
+               $op->addWikiTextAsContent( 'a' );
+       }
+
        /**
         * @covers OutputPage::addWikiMsg
         */