Apply content wrapping in ParserOutput::getText()

Instead of applying wrapping the the parser and unwrapping in
ParserOutput::getText(), turn this around and apply wrapping in getText(),
and only if desired.

This avoids search&replace logic for unwrapping, and it also makes it a lot
easier to merge the output of multiple slots for MCR output.

This changes behavior in two hopefully irrelevant ways:
1) the limit report comments will be inside the wrapper div, instead of
following it.
2) if HTML with a wrapper div is explicitly injected into a ParserOutput
object, it will not be possible to unwrap the text.

Bug: T174035
Change-Id: I1641b7995af9bd297f1acd610d583fbf874f34e0

diff --git a/includes/api/ApiParse.php b/includes/api/ApiParse.php
index 3a60471..5c25b5a 100644 (file)
--- a/includes/api/ApiParse.php
+++ b/includes/api/ApiParse.php
@@ -341,7 +341,7 @@ class ApiParse extends ApiBase {
                        $result_array['text'] = $p_result->getText( [
                                'allowTOC' => !$params['disabletoc'],
                                'enableSectionEditLinks' => !$params['disableeditsection'],
-                               'unwrap' => $params['wrapoutputclass'] === '',
+                               'wrapperDivClass' => $params['wrapoutputclass'],
                                'deduplicateStyles' => !$params['disablestylededuplication'],
                        ] );
                        $result_array[ApiResult::META_BC_SUBELEMENTS][] = 'text';

diff --git a/includes/content/TextContent.php b/includes/content/TextContent.php
index 20bce37..0198a0d 100644 (file)
--- a/includes/content/TextContent.php
+++ b/includes/content/TextContent.php
@@ -253,6 +253,7 @@ class TextContent extends AbstractContent {
                        $html = '';
                }
 
+               $output->clearWrapperDivClass();
                $output->setText( $html );
        }
 

diff --git a/includes/parser/Parser.php b/includes/parser/Parser.php
index 6bee169..47f81e6 100644 (file)
--- a/includes/parser/Parser.php
+++ b/includes/parser/Parser.php
@@ -517,7 +517,7 @@ class Parser {
                # with CSS (T37247)
                $class = $this->mOptions->getWrapOutputClass();
                if ( $class !== false && !$this->mOptions->getInterfaceMessage() ) {
-                       $text = Html::rawElement( 'div', [ 'class' => $class ], $text );
+                       $this->mOutput->addWrapperDivClass( $class );
                }
 
                $this->mOutput->setText( $text );

diff --git a/includes/parser/ParserOutput.php b/includes/parser/ParserOutput.php
index 182648a..fe9913d 100644 (file)
--- a/includes/parser/ParserOutput.php
+++ b/includes/parser/ParserOutput.php
@@ -212,6 +212,11 @@ class ParserOutput extends CacheTime {
        /** @var int|null Assumed rev ID for {{REVISIONID}} if no revision is set */
        private $mSpeculativeRevId;
 
+       /** string CSS classes to use for the wrapping div, stored in the array keys.
+        * If no class is given, no wrapper is added.
+        */
+       private $mWrapperDivClasses = [];
+
        /** @var int Upper bound of expiry based on parse duration */
        private $mMaxAdaptiveExpiry = INF;
 
@@ -258,7 +263,12 @@ class ParserOutput extends CacheTime {
         *  - enableSectionEditLinks: (bool) Include section edit links, assuming
         *     section edit link tokens are present in the HTML. Default is true,
         *     but might be statefully overridden.
-        *  - unwrap: (bool) Remove a wrapping mw-parser-output div. Default is false.
+        *  - unwrap: (bool) Return text without a wrapper div. Default is false,
+        *    meaning a wrapper div will be added if getWrapperDivClass() returns
+        *    a non-empty string.
+        *  - wrapperDivClass: (string) Wrap the output in a div and apply the given
+        *    CSS class to that div. This overrides the output of getWrapperDivClass().
+        *    Setting this to an empty string has the same effect as 'unwrap' => true.
         *  - deduplicateStyles: (bool) When true, which is the default, `<style>`
         *    tags with the `data-mw-deduplicate` attribute set are deduplicated by
         *    value of the attribute: all but the first will be replaced by `<link
@@ -273,28 +283,14 @@ class ParserOutput extends CacheTime {
                        'enableSectionEditLinks' => true,
                        'unwrap' => false,
                        'deduplicateStyles' => true,
+                       'wrapperDivClass' => $this->getWrapperDivClass(),
                ];
                $text = $this->mText;
 
                Hooks::runWithoutAbort( 'ParserOutputPostCacheTransform', [ $this, &$text, &$options ] );
 
-               if ( $options['unwrap'] !== false ) {
-                       $start = Html::openElement( 'div', [
-                               'class' => 'mw-parser-output'
-                       ] );
-                       $startLen = strlen( $start );
-                       $end = Html::closeElement( 'div' );
-                       $endPos = strrpos( $text, $end );
-                       $endLen = strlen( $end );
-
-                       if ( substr( $text, 0, $startLen ) === $start && $endPos !== false
-                               // if the closing div is followed by real content, bail out of unwrapping
-                               && preg_match( '/^(?>\s*<!--.*?-->)*\s*$/s', substr( $text, $endPos + $endLen ) )
-                       ) {
-                               $text = substr( $text, $startLen );
-                               $text = substr( $text, 0, $endPos - $startLen )
-                                       . substr( $text, $endPos - $startLen + $endLen );
-                       }
+               if ( $options['wrapperDivClass'] !== '' && !$options['unwrap'] ) {
+                       $text = Html::rawElement( 'div', [ 'class' => $options['wrapperDivClass'] ], $text );
                }
 
                if ( $options['enableSectionEditLinks'] ) {
@@ -365,6 +361,34 @@ class ParserOutput extends CacheTime {
                return $text;
        }
 
+       /**
+        * Add a CSS class to use for the wrapping div. If no class is given, no wrapper is added.
+        *
+        * @param string $class
+        */
+       public function addWrapperDivClass( $class ) {
+               $this->mWrapperDivClasses[$class] = true;
+       }
+
+       /**
+        * Clears the CSS class to use for the wrapping div, effectively disabling the wrapper div
+        * until addWrapperDivClass() is called.
+        */
+       public function clearWrapperDivClass() {
+               $this->mWrapperDivClasses = [];
+       }
+
+       /**
+        * Returns the class (or classes) to be used with the wrapper div for this otuput.
+        * If there is no wrapper class given, no wrapper div should be added.
+        * The wrapper div is added automatically by getText().
+        *
+        * @return string
+        */
+       public function getWrapperDivClass() {
+               return implode( ' ', array_keys( $this->mWrapperDivClasses ) );
+       }
+
        /**
         * @param int $id
         * @since 1.28

diff --git a/tests/phpunit/includes/api/ApiParseTest.php b/tests/phpunit/includes/api/ApiParseTest.php
index 8a40266..7f2c1a6 100644 (file)
--- a/tests/phpunit/includes/api/ApiParseTest.php
+++ b/tests/phpunit/includes/api/ApiParseTest.php
@@ -77,11 +77,15 @@ class ApiParseTest extends ApiTestCase {
                        $expectedEnd = "</div>";
                        $this->assertSame( $expectedEnd, substr( $html, -strlen( $expectedEnd ) ) );
 
+                       $unexpectedEnd = '#<!-- \nNewPP limit report|' .
+                               '<!--\nTransclusion expansion time report#s';
+                       $this->assertNotRegExp( $unexpectedEnd, $html );
+
                        $html = substr( $html, 0, strlen( $html ) - strlen( $expectedEnd ) );
                } else {
                        $expectedEnd = '#\n<!-- \nNewPP limit report\n(?>.+?\n-->)\n' .
                                '<!--\nTransclusion expansion time report \(%,ms,calls,template\)\n(?>.*?\n-->)\n' .
-                               '</div>(\n<!-- Saved in parser cache (?>.*?\n -->)\n)?$#s';
+                               '(\n<!-- Saved in parser cache (?>.*?\n -->)\n)?</div>$#s';
                        $this->assertRegExp( $expectedEnd, $html );
 
                        $html = preg_replace( $expectedEnd, '', $html );

diff --git a/tests/phpunit/includes/parser/ParserMethodsTest.php b/tests/phpunit/includes/parser/ParserMethodsTest.php
index d2ed441..497c6b5 100644 (file)
--- a/tests/phpunit/includes/parser/ParserMethodsTest.php
+++ b/tests/phpunit/includes/parser/ParserMethodsTest.php
@@ -180,6 +180,18 @@ class ParserMethodsTest extends MediaWikiLangTestCase {
                ];
        }
 
+       public function testWrapOutput() {
+               global $wgParser;
+               $title = Title::newFromText( 'foo' );
+               $po = new ParserOptions();
+               $wgParser->parse( 'Hello World', $title, $po );
+               $text = $wgParser->getOutput()->getText();
+
+               $this->assertContains( 'Hello World', $text );
+               $this->assertContains( '<div', $text );
+               $this->assertContains( 'class="mw-parser-output"', $text );
+       }
+
        // @todo Add tests for cleanSig() / cleanSigInSig(), getSection(),
        // replaceSection(), getPreloadText()
 }

diff --git a/tests/phpunit/includes/parser/ParserOutputTest.php b/tests/phpunit/includes/parser/ParserOutputTest.php
index b08ba6c..439b24d 100644 (file)
--- a/tests/phpunit/includes/parser/ParserOutputTest.php
+++ b/tests/phpunit/includes/parser/ParserOutputTest.php
@@ -89,6 +89,59 @@ class ParserOutputTest extends MediaWikiTestCase {
                $this->assertArrayNotHasKey( 'foo', $properties );
        }
 
+       /**
+        * @covers ParserOutput::getWrapperDivClass
+        * @covers ParserOutput::addWrapperDivClass
+        * @covers ParserOutput::clearWrapperDivClass
+        * @covers ParserOutput::getText
+        */
+       public function testWrapperDivClass() {
+               $po = new ParserOutput();
+
+               $po->setText( 'Kittens' );
+               $this->assertContains( 'Kittens', $po->getText() );
+               $this->assertNotContains( '<div', $po->getText() );
+               $this->assertSame( 'Kittens', $po->getRawText() );
+
+               $po->addWrapperDivClass( 'foo' );
+               $text = $po->getText();
+               $this->assertContains( 'Kittens', $text );
+               $this->assertContains( '<div', $text );
+               $this->assertContains( 'class="foo"', $text );
+
+               $po->addWrapperDivClass( 'bar' );
+               $text = $po->getText();
+               $this->assertContains( 'Kittens', $text );
+               $this->assertContains( '<div', $text );
+               $this->assertContains( 'class="foo bar"', $text );
+
+               $po->addWrapperDivClass( 'bar' ); // second time does nothing, no "foo bar bar".
+               $text = $po->getText( [ 'unwrap' => true ] );
+               $this->assertContains( 'Kittens', $text );
+               $this->assertNotContains( '<div', $text );
+               $this->assertNotContains( 'class="foo bar"', $text );
+
+               $text = $po->getText( [ 'wrapperDivClass' => '' ] );
+               $this->assertContains( 'Kittens', $text );
+               $this->assertNotContains( '<div', $text );
+               $this->assertNotContains( 'class="foo bar"', $text );
+
+               $text = $po->getText( [ 'wrapperDivClass' => 'xyzzy' ] );
+               $this->assertContains( 'Kittens', $text );
+               $this->assertContains( '<div', $text );
+               $this->assertContains( 'class="xyzzy"', $text );
+               $this->assertNotContains( 'class="foo bar"', $text );
+
+               $text = $po->getRawText();
+               $this->assertSame( 'Kittens', $text );
+
+               $po->clearWrapperDivClass();
+               $text = $po->getText();
+               $this->assertContains( 'Kittens', $text );
+               $this->assertNotContains( '<div', $text );
+               $this->assertNotContains( 'class="foo bar"', $text );
+       }
+
        /**
         * @covers ParserOutput::getText
         * @dataProvider provideGetText
@@ -111,7 +164,7 @@ class ParserOutputTest extends MediaWikiTestCase {
        public static function provideGetText() {
                // phpcs:disable Generic.Files.LineLength
                $text = <<<EOF
-<div class="mw-parser-output"><p>Test document.
+<p>Test document.
 </p>
 <mw:toc><div id="toc" class="toc"><div class="toctitle"><h2>Contents</h2></div>
 <ul>
@@ -136,7 +189,7 @@ class ParserOutputTest extends MediaWikiTestCase {
 </p>
 <h2><span class="mw-headline" id="Section_3">Section 3</span><mw:editsection page="Test Page" section="4">Section 3</mw:editsection></h2>
 <p>Three
-</p></div>
+</p>
 EOF;
 
                $dedupText = <<<EOF
@@ -155,7 +208,7 @@ EOF;
                return [
                        'No options' => [
                                [], $text, <<<EOF
-<div class="mw-parser-output"><p>Test document.
+<p>Test document.
 </p>
 <div id="toc" class="toc"><div class="toctitle"><h2>Contents</h2></div>
 <ul>
@@ -180,12 +233,12 @@ EOF;
 </p>
 <h2><span class="mw-headline" id="Section_3">Section 3</span><span class="mw-editsection"><span class="mw-editsection-bracket">[</span><a href="/w/index.php?title=Test_Page&amp;action=edit&amp;section=4" title="Edit section: Section 3">edit</a><span class="mw-editsection-bracket">]</span></span></h2>
 <p>Three
-</p></div>
+</p>
 EOF
                        ],
                        'Disable section edit links' => [
                                [ 'enableSectionEditLinks' => false ], $text, <<<EOF
-<div class="mw-parser-output"><p>Test document.
+<p>Test document.
 </p>
 <div id="toc" class="toc"><div class="toctitle"><h2>Contents</h2></div>
 <ul>
@@ -210,11 +263,11 @@ EOF
 </p>
 <h2><span class="mw-headline" id="Section_3">Section 3</span></h2>
 <p>Three
-</p></div>
+</p>
 EOF
                        ],
-                       'Disable TOC' => [
-                               [ 'allowTOC' => false ], $text, <<<EOF
+                       'Disable TOC, but wrap' => [
+                               [ 'allowTOC' => false, 'wrapperDivClass' => 'mw-parser-output' ], $text, <<<EOF
 <div class="mw-parser-output"><p>Test document.
 </p>
 
@@ -231,44 +284,6 @@ EOF
 <p>Three
 </p></div>
 EOF
-                       ],
-                       'Unwrap text' => [
-                               [ 'unwrap' => true ], $text, <<<EOF
-<p>Test document.
-</p>
-<div id="toc" class="toc"><div class="toctitle"><h2>Contents</h2></div>
-<ul>
-<li class="toclevel-1 tocsection-1"><a href="#Section_1"><span class="tocnumber">1</span> <span class="toctext">Section 1</span></a></li>
-<li class="toclevel-1 tocsection-2"><a href="#Section_2"><span class="tocnumber">2</span> <span class="toctext">Section 2</span></a>
-<ul>
-<li class="toclevel-2 tocsection-3"><a href="#Section_2.1"><span class="tocnumber">2.1</span> <span class="toctext">Section 2.1</span></a></li>
-</ul>
-</li>
-<li class="toclevel-1 tocsection-4"><a href="#Section_3"><span class="tocnumber">3</span> <span class="toctext">Section 3</span></a></li>
-</ul>
-</div>
-
-<h2><span class="mw-headline" id="Section_1">Section 1</span><span class="mw-editsection"><span class="mw-editsection-bracket">[</span><a href="/w/index.php?title=Test_Page&amp;action=edit&amp;section=1" title="Edit section: Section 1">edit</a><span class="mw-editsection-bracket">]</span></span></h2>
-<p>One
-</p>
-<h2><span class="mw-headline" id="Section_2">Section 2</span><span class="mw-editsection"><span class="mw-editsection-bracket">[</span><a href="/w/index.php?title=Test_Page&amp;action=edit&amp;section=2" title="Edit section: Section 2">edit</a><span class="mw-editsection-bracket">]</span></span></h2>
-<p>Two
-</p>
-<h3><span class="mw-headline" id="Section_2.1">Section 2.1</span><span class="mw-editsection"><span class="mw-editsection-bracket">[</span><a href="/w/index.php?title=Test_Page&amp;action=edit&amp;section=3" title="Edit section: Section 2.1">edit</a><span class="mw-editsection-bracket">]</span></span></h3>
-<p>Two point one
-</p>
-<h2><span class="mw-headline" id="Section_3">Section 3</span><span class="mw-editsection"><span class="mw-editsection-bracket">[</span><a href="/w/index.php?title=Test_Page&amp;action=edit&amp;section=4" title="Edit section: Section 3">edit</a><span class="mw-editsection-bracket">]</span></span></h2>
-<p>Three
-</p>
-EOF
-                       ],
-                       'Unwrap without a mw-parser-output wrapper' => [
-                               [ 'unwrap' => true ], '<div class="foobar">Content</div>', '<div class="foobar">Content</div>'
-                       ],
-                       'Unwrap with extra comment at end' => [
-                               [ 'unwrap' => true ], '<div class="mw-parser-output"><p>Test document.</p></div>
-<!-- Saved in parser cache... -->', '<p>Test document.</p>
-<!-- Saved in parser cache... -->'
                        ],
                        'Style deduplication' => [
                                [], $dedupText, <<<EOF