From: C. Scott Ananian <cscott@cscott.net>
Date: Tue, 25 Sep 2018 13:06:12 +0000 (-0400)
Subject: Don't wrap output added by OutputPage::addWikiText*()
X-Git-Tag: 1.34.0-rc.0~3985^2
X-Git-Url: http://git.heureux-cyclage.org/?p=lhc%2Fweb%2Fwiklou.git;a=commitdiff_plain;h=300b91a1eccecb5f9c4e36126222edfddb33474b

Don't wrap output added by OutputPage::addWikiText*()

There are three methods affected: `OutputPage::addWikiTextTidy()`,
`OutputPage::addWikiTextTitleTidy()`, and
`OutputPage::addWikiTextWithTitle()`.

There's a special case in Parser.php which adds the wrapper class from
ParserOptions to the ParserOutput only if "interface mode" is off; the
affected methods default to adding output in "content language" mode
(not "interface language" mode), but they seem to be used for
"interface messages in the content language" (rare) and so should also
be unwrapped.  This would make all the `OutputPage::addWikiText*()`
methods consistent.

The `OutputPage::addWikiTextTidy()` method is only used once in the WMF
repositories, where it is used to insert an interface message in the
content language:

https://gerrit.wikimedia.org/g/mediawiki/extensions/ProofreadPage/+/91cd2a928f53e12f7c41bc59f1085b5415632511/SpecialProofreadPages.php#40

The `OutputPage::addWikiTextWithTitle()` method is used by no one:

https://codesearch.wmflabs.org/search/?q=addWikiTextWithTitle%5C(

The `OutputPage::addWikiTextTitleTidy()` method is used only in core:

https://gerrit.wikimedia.org/g/mediawiki/core/+/3888c001a1dbc04f1fd6bb51328b1cb1296c02f6/includes/EditPage.php#2669

It seems clear that the output in this case is intended to be
unwrapped as well (the codepath adds its own explicit wrapper).

Ia58910164baaca608cea3b24333b7d13ed773339 will add additional
documentation to clarify the distinction between the different
OutputPage::addWikiText*() methods, but I felt it safer to make
this particular change first as a standalone patch, just in case
it had unexpected side effects or merited further discussion.

Change-Id: I3e5b598d358819191562b56d40ebf1cb6f3cda41
---

diff --git a/includes/OutputPage.php b/includes/OutputPage.php
index 99a4c2b895..2bfccda265 100644
--- a/includes/OutputPage.php
+++ b/includes/OutputPage.php
@@ -1766,7 +1766,8 @@ class OutputPage extends ContextSource {
 	}
 
 	/**
-	 * Add wikitext with a custom Title object
+	 * Add wikitext with a custom Title object.
+	 * Output is unwrapped.
 	 *
 	 * @param string $text Wikitext
 	 * @param Title $title
@@ -1793,6 +1794,7 @@ class OutputPage extends ContextSource {
 
 		$this->addParserOutput( $parserOutput, [
 			'enableSectionEditLinks' => false,
+			'wrapperDivClass' => '',
 		] );
 	}
 
diff --git a/tests/phpunit/includes/OutputPageTest.php b/tests/phpunit/includes/OutputPageTest.php
index e7b7c76cf6..9fedc4efc2 100644
--- a/tests/phpunit/includes/OutputPageTest.php
+++ b/tests/phpunit/includes/OutputPageTest.php
@@ -1379,6 +1379,8 @@ class OutputPageTest extends MediaWikiTestCase {
 	 * @covers OutputPage::addWikiText
 	 * @covers OutputPage::addWikiTextWithTitle
 	 * @covers OutputPage::addWikiTextTitle
+	 * @covers OutputPage::addWikiTextTidy
+	 * @covers OutputPage::addWikiTextTitleTidy
 	 * @covers OutputPage::getHTML
 	 */
 	public function testAddWikiText( $method, array $args, $expected ) {
@@ -1411,7 +1413,7 @@ class OutputPageTest extends MediaWikiTestCase {
 					'* Not a list',
 				], 'Non-interface' => [
 					[ "'''Bold'''", true, false ],
-					"<div class=\"mw-parser-output\"><p><b>Bold</b>\n</p></div>",
+					"<p><b>Bold</b>\n</p>",
 				], 'No section edit links' => [
 					[ '== Title ==' ],
 					"<h2><span class=\"mw-headline\" id=\"Title\">Title</span></h2>\n",
@@ -1420,10 +1422,34 @@ class OutputPageTest extends MediaWikiTestCase {
 			'addWikiTextWithTitle' => [
 				'With title at start' => [
 					[ '* {{PAGENAME}}', Title::newFromText( 'Talk:Some page' ) ],
-					"<div class=\"mw-parser-output\"><ul><li>Some page</li></ul>\n</div>",
+					"<ul><li>Some page</li></ul>\n",
+				], 'With title at start' => [
+					[ '* {{PAGENAME}}', Title::newFromText( 'Talk:Some page' ), false ],
+					"* Some page",
+				],
+			],
+			'addWikiTextTidy' => [
+				'SpecialNewimages' => [
+					[ "<p lang='en' dir='ltr'>\nMy message" ],
+					'<p lang="en" dir="ltr">' . "\nMy message\n</p>"
+				], 'List at start' => [
+					[ '* List' ],
+					"<ul><li>List</li></ul>\n",
+				], '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' ) ],
+					"<ul><li>Some page</li></ul>\n",
 				], 'With title at start' => [
 					[ '* {{PAGENAME}}', Title::newFromText( 'Talk:Some page' ), false ],
-					"<div class=\"mw-parser-output\">* Some page</div>",
+					"<p>* Some page</p>",
+				], 'EditPage' => [
+					[ "<div class='mw-editintro'>{{PAGENAME}}", Title::newFromText( 'Talk:Some page' ) ],
+					'<div class="mw-editintro">' . "Some page\n</div>"
 				],
 			],
 		];
@@ -1439,6 +1465,16 @@ 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 ];
+			$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 ];
+			$tests['addWikiTextTitle']["$key (addWikiTextTitle)"] =
+				array_merge( [ $args ], array_slice( $val, 1 ) );
+		}
 
 		// We have to reformat our array to match what PHPUnit wants
 		$ret = [];
@@ -1462,8 +1498,6 @@ class OutputPageTest extends MediaWikiTestCase {
 		$op->addWikiText( 'a' );
 	}
 
-	// @todo How should we cover the Tidy variants?
-
 	/**
 	 * @covers OutputPage::addParserOutputMetadata
 	 */