From: C. Scott Ananian Date: Fri, 21 Sep 2018 16:24:57 +0000 (-0400) Subject: Deprecate and rename OutputPage::addWikiText* methods X-Git-Tag: 1.34.0-rc.0~3836 X-Git-Url: http://git.cyclocoop.org/%22%2C%20generer_url_ecrire%28?a=commitdiff_plain;h=7af52285f735b224ee60e6565fb25364da4e318e;p=lhc%2Fweb%2Fwiklou.git 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 ec34acb30b..d929453185 100644 --- 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 dd2f5acf4a..5801a29ac9 100644 --- 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 054636ec7c..f9c293cdec 100644 --- 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'''" ], "

Bold\n

", @@ -1435,6 +1445,7 @@ class OutputPageTest extends MediaWikiTestCase { ], ], 'addWikiTextWithTitle' => [ + // Untidied; this API is deprecated 'With title at start' => [ [ '* {{PAGENAME}}', Title::newFromText( 'Talk:Some page' ) ], "\n", @@ -1443,7 +1454,36 @@ class OutputPageTest extends MediaWikiTestCase { "* Some page", ], ], - 'addWikiTextTidy' => [ + 'addWikiTextAsInterface' => [ + // Preferred interface: output is tidied + 'Simple wikitext' => [ + [ "'''Bold'''" ], + "

Bold\n

", + ], 'Untidy wikitext' => [ + [ "Bold" ], + "

Bold\n

", + ], 'List at start' => [ + [ '* List' ], + "\n", + ], 'List not at start' => [ + [ '* Not a list', false ], + '

* Not a list

', + ], 'No section edit links' => [ + [ '== Title ==' ], + "

Title

\n", + ], 'With title at start' => [ + [ '* {{PAGENAME}}', true, Title::newFromText( 'Talk:Some page' ) ], + "\n", + ], 'With title at start' => [ + [ '* {{PAGENAME}}', false, Title::newFromText( 'Talk:Some page' ), false ], + "

* Some page

", + ], 'Untidy input' => [ + [ '{{PAGENAME}}', true, Title::newFromText( 'Talk:Some page' ) ], + "

Some page\n

", + ], + ], + 'addWikiTextAsContent' => [ + // Preferred interface: output is tidied 'SpecialNewimages' => [ [ "

\nMy message" ], '

' . "\nMy message\n

" @@ -1453,17 +1493,14 @@ class OutputPageTest extends MediaWikiTestCase { ], 'List not at start' => [ [ '* Not a list', false ], '

* Not a list

', - ], - ], - 'addWikiTextTitleTidy' => [ - 'With title at start' => [ - [ '* {{PAGENAME}}', Title::newFromText( 'Talk:Some page' ) ], + ], 'With title at start' => [ + [ '* {{PAGENAME}}', true, Title::newFromText( 'Talk:Some page' ) ], "
  • Some page
\n", ], 'With title at start' => [ - [ '* {{PAGENAME}}', Title::newFromText( 'Talk:Some page' ), false ], + [ '* {{PAGENAME}}', false, Title::newFromText( 'Talk:Some page' ), false ], "

* Some page

", ], 'EditPage' => [ - [ "
{{PAGENAME}}", Title::newFromText( 'Talk:Some page' ) ], + [ "
{{PAGENAME}}", true, Title::newFromText( 'Talk:Some page' ) ], '
' . "Some page\n
" ], ], @@ -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 */