Fix OutputPage::parseInternal() by stripping <div> wrapper
authorC. Scott Ananian <cscott@cscott.net>
Fri, 26 Oct 2018 14:05:34 +0000 (10:05 -0400)
committerC. Scott Ananian <cscott@cscott.net>
Fri, 26 Oct 2018 15:20:26 +0000 (11:20 -0400)
We should probably strip the <div> wrapper in OutputPage::parse() as
well: this behavior was added in 1.30, but it only applies when
$interface is false.  However, that requires a more careful audit
(a lot more places call parse() than parseInline()) and so I'll defer
that for now.

Change-Id: Iad5412f03af29c04deb653969dd71f6c86f0ae50

includes/OutputPage.php
tests/phpunit/includes/OutputPageTest.php

index ca3f6a3..97d9d83 100644 (file)
@@ -2091,6 +2091,9 @@ class OutputPage extends ContextSource {
        /**
         * Parse wikitext and return the HTML.
         *
+        * @todo The output is wrapped in a <div> iff $interface is false; it's
+        * probably best to always strip the wrapper.
+        *
         * @param string $text
         * @param bool $linestart Is this the start of a line?
         * @param bool $interface Use interface language (instead of content language) while parsing
@@ -2101,6 +2104,45 @@ class OutputPage extends ContextSource {
         * @return string HTML
         */
        public function parse( $text, $linestart = true, $interface = false, $language = null ) {
+               return $this->parseInternal(
+                       $text, $linestart, $interface, $language
+               )->getText( [
+                       'enableSectionEditLinks' => false,
+               ] );
+       }
+
+       /**
+        * Parse wikitext, strip paragraph wrapper, and return the HTML.
+        *
+        * @param string $text
+        * @param bool $linestart Is this the start of a line?
+        * @param bool $interface Use interface language (instead of content language) while parsing
+        *   language sensitive magic words like GRAMMAR and PLURAL
+        * @return string HTML
+        */
+       public function parseInline( $text, $linestart = true, $interface = false ) {
+               $parsed = $this->parseInternal(
+                       $text, $linestart, $interface, /*language*/null
+               )->getText( [
+                       'enableSectionEditLinks' => false,
+                       'wrapperDivClass' => '', /* no wrapper div */
+               ] );
+               return Parser::stripOuterParagraph( $parsed );
+       }
+
+       /**
+        * Parse wikitext and return the HTML (internal implementation helper)
+        *
+        * @param string $text
+        * @param bool $linestart Is this the start of a line?
+        * @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 Language|null $language Target language object, will override $interface
+        * @throws MWException
+        * @return ParserOutput
+        */
+       private function parseInternal( $text, $linestart, $interface, $language ) {
                global $wgParser;
 
                if ( is_null( $this->getTitle() ) ) {
@@ -2127,26 +2169,7 @@ class OutputPage extends ContextSource {
                        $popts->setTargetLanguage( $oldLang );
                }
 
-               return $parserOutput->getText( [
-                       'enableSectionEditLinks' => false,
-               ] );
-       }
-
-       /**
-        * Parse wikitext, strip paragraphs, and return the HTML.
-        *
-        * @todo This doesn't work as expected at all.  If $interface is false, there's always a
-        * wrapping <div>, so stripOuterParagraph() does nothing.
-        *
-        * @param string $text
-        * @param bool $linestart Is this the start of a line?
-        * @param bool $interface Use interface language (instead of content language) while parsing
-        *   language sensitive magic words like GRAMMAR and PLURAL
-        * @return string HTML
-        */
-       public function parseInline( $text, $linestart = true, $interface = false ) {
-               $parsed = $this->parse( $text, $linestart, $interface );
-               return Parser::stripOuterParagraph( $parsed );
+               return $parserOutput;
        }
 
        /**
index 90413c4..cd20bb2 100644 (file)
@@ -1826,28 +1826,44 @@ class OutputPageTest extends MediaWikiTestCase {
 
        public function provideParse() {
                return [
-                       'List at start of line' => [
-                               [ '* List' ],
+                       'List at start of line (content)' => [
+                               [ '* List', true, false ],
                                "<div class=\"mw-parser-output\"><ul><li>List</li></ul>\n</div>",
+                               "<ul><li>List</li></ul>\n",
                        ],
-                       'List not at start' => [
-                               [ "* ''Not'' list", false ],
+                       'List at start of line (interface)' => [
+                               [ '* List', true, true ],
+                               "<ul><li>List</li></ul>\n",
+                       ],
+                       'List not at start (content)' => [
+                               [ "* ''Not'' list", false, false ],
                                '<div class="mw-parser-output">* <i>Not</i> list</div>',
+                               '* <i>Not</i> list',
+                       ],
+                       'List not at start (interface)' => [
+                               [ "* ''Not'' list", false, true ],
+                               '* <i>Not</i> list',
                        ],
-                       'Interface' => [
+                       'Interface message' => [
                                [ "''Italic''", true, true ],
                                "<p><i>Italic</i>\n</p>",
                                '<i>Italic</i>',
                        ],
-                       'formatnum' => [
-                               [ '{{formatnum:123456.789}}' ],
+                       'formatnum (content)' => [
+                               [ '{{formatnum:123456.789}}', true, false ],
                                "<div class=\"mw-parser-output\"><p>123,456.789\n</p></div>",
+                               "123,456.789",
+                       ],
+                       'formatnum (interface)' => [
+                               [ '{{formatnum:123456.789}}', true, true ],
+                               "<p>123,456.789\n</p>",
+                               "123,456.789",
                        ],
-                       'Language' => [
+                       'Language (content)' => [
                                [ '{{formatnum:123456.789}}', true, false, Language::factory( 'is' ) ],
                                "<div class=\"mw-parser-output\"><p>123.456,789\n</p></div>",
                        ],
-                       'Language with interface' => [
+                       'Language (interface)' => [
                                [ '{{formatnum:123456.789}}', true, true, Language::factory( 'is' ) ],
                                "<p>123.456,789\n</p>",
                                '123.456,789',
@@ -1856,6 +1872,8 @@ class OutputPageTest extends MediaWikiTestCase {
                                [ '== Header ==' ],
                                '<div class="mw-parser-output"><h2><span class="mw-headline" id="Header">' .
                                        "Header</span></h2>\n</div>",
+                               '<h2><span class="mw-headline" id="Header">Header</span></h2>' .
+                                       "\n",
                        ]
                ];
        }