Anomie has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/399907 )

Change subject: Add 'unwrap' ParserOutput post-cache transform
......................................................................

Add 'unwrap' ParserOutput post-cache transform

And deprecate passing false for ParserOptions::setWrapOutputClass().

There are three cases for the Parser wrapper: the default
mw-parser-output, a custom wrapper, or no wrapper. As things currently
stand, we have to fragment the parser cache on each of these options,
which uses a nontrival amount of storage space (T167784).

Ideally we'd do all the wrapping as a post-cache transform, but
TemplateStyles needs to know the wrapper in use in order to properly
prefix its CSS rules (that's why we added the wrapper in the first
place). So, second best option is to make *un*wrapping be a post-cache
transform and make "custom wrapper" be uncacheable.

This patch does the first bit (unwrapping as a post-cache transform),
and a followup will do the second part once the deprecation process is
satisfied.

Bug: T181846
Change-Id: Iba16e78c41be992467101e7d83e9c3134765b101
---
M RELEASE-NOTES-1.31
M includes/Message.php
M includes/api/ApiParse.php
M includes/cache/MessageCache.php
M includes/installer/Installer.php
M includes/parser/ParserOptions.php
M includes/parser/ParserOutput.php
M tests/parser/ParserTestRunner.php
M tests/phpunit/includes/ExtraParserTest.php
M tests/phpunit/includes/parser/ParserOptionsTest.php
M tests/phpunit/includes/parser/ParserOutputTest.php
M tests/phpunit/includes/parser/TagHooksTest.php
12 files changed, 94 insertions(+), 41 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/07/399907/1

diff --git a/RELEASE-NOTES-1.31 b/RELEASE-NOTES-1.31
index 1a1a9f7..31d3f51 100644
--- a/RELEASE-NOTES-1.31
+++ b/RELEASE-NOTES-1.31
@@ -146,6 +146,8 @@
   * WatchedItem::IGNORE_USER_RIGHTS
   * WatchedItem::CHECK_USER_RIGHTS
   * WatchedItem::DEPRECATED_USAGE_TIMESTAMP
+* Passing false to ParserOptions::setWrapOutputClass() is deprecated. Use the
+  'unwrap' transform to ParserOutput::getText() instead.
 
 == Compatibility ==
 MediaWiki 1.31 requires PHP 5.5.9 or later. There is experimental support for
diff --git a/includes/Message.php b/includes/Message.php
index e55eaaf..fac9a59 100644
--- a/includes/Message.php
+++ b/includes/Message.php
@@ -1245,7 +1245,14 @@
                );
 
                return $out instanceof ParserOutput
-                       ? $out->getText( [ 'enableSectionEditLinks' => false ] )
+                       ? $out->getText( [
+                               'enableSectionEditLinks' => false,
+                               // Wrapping messages in an extra <div> is 
probably not expected. If
+                               // they're outside the content area they 
probably shouldn't be
+                               // targeted by CSS that's targeting the parser 
output, and if
+                               // they're inside they already are from the 
outer div.
+                               'unwrap' => true,
+                       ] )
                        : $out;
        }
 
diff --git a/includes/api/ApiParse.php b/includes/api/ApiParse.php
index ec015da..d512397 100644
--- a/includes/api/ApiParse.php
+++ b/includes/api/ApiParse.php
@@ -346,6 +346,7 @@
                        $result_array['text'] = $p_result->getText( [
                                'allowTOC' => !$params['disabletoc'],
                                'enableSectionEditLinks' => 
!$params['disableeditsection'],
+                               'unwrap' => $params['wrapoutputclass'] === '',
                        ] );
                        $result_array[ApiResult::META_BC_SUBELEMENTS][] = 
'text';
                }
@@ -540,9 +541,9 @@
                if ( $params['disabletidy'] ) {
                        $popts->setTidy( false );
                }
-               $popts->setWrapOutputClass(
-                       $params['wrapoutputclass'] === '' ? false : 
$params['wrapoutputclass']
-               );
+               if ( $params['wrapoutputclass'] !== '' ) {
+                       $popts->setWrapOutputClass( $params['wrapoutputclass'] 
);
+               }
 
                $reset = null;
                $suppressCache = false;
diff --git a/includes/cache/MessageCache.php b/includes/cache/MessageCache.php
index 768f980..9f944b5 100644
--- a/includes/cache/MessageCache.php
+++ b/includes/cache/MessageCache.php
@@ -193,7 +193,6 @@
                                $po = ParserOptions::newFromAnon();
                                $po->setEditSection( false );
                                $po->setAllowUnsafeRawHtml( false );
-                               $po->setWrapOutputClass( false );
                                return $po;
                        }
 
@@ -203,11 +202,6 @@
                        // from malicious sources. As a precaution, disable
                        // the <html> parser tag when parsing messages.
                        $this->mParserOptions->setAllowUnsafeRawHtml( false );
-                       // Wrapping messages in an extra <div> is probably not 
expected. If
-                       // they're outside the content area they probably 
shouldn't be
-                       // targeted by CSS that's targeting the parser output, 
and if
-                       // they're inside they already are from the outer div.
-                       $this->mParserOptions->setWrapOutputClass( false );
                }
 
                return $this->mParserOptions;
diff --git a/includes/installer/Installer.php b/includes/installer/Installer.php
index 2bb15b8..c596c5c 100644
--- a/includes/installer/Installer.php
+++ b/includes/installer/Installer.php
@@ -447,7 +447,6 @@
                $this->parserTitle = Title::newFromText( 'Installer' );
                $this->parserOptions = new ParserOptions( $wgUser ); // 
language will be wrong :(
                $this->parserOptions->setEditSection( false );
-               $this->parserOptions->setWrapOutputClass( false );
                // Don't try to access DB before user language is initialised
                $this->setParserLanguage( Language::factory( 'en' ) );
        }
@@ -690,6 +689,7 @@
                        $out = $wgParser->parse( $text, $this->parserTitle, 
$this->parserOptions, $lineStart );
                        $html = $out->getText( [
                                'enableSectionEditLinks' => false,
+                               'unwrap' => true,
                        ] );
                } catch ( MediaWiki\Services\ServiceDisabledException $e ) {
                        $html = '<!--DB access attempted during parse-->  ' . 
htmlspecialchars( $text );
diff --git a/includes/parser/ParserOptions.php 
b/includes/parser/ParserOptions.php
index f99089b..a40ef12 100644
--- a/includes/parser/ParserOptions.php
+++ b/includes/parser/ParserOptions.php
@@ -781,6 +781,7 @@
         * CSS class to use to wrap output from Parser::parse()
         * @since 1.30
         * @param string|bool $className Set false to disable wrapping.
+        *   Passing false is deprecated since MediaWiki 1.31
         * @return string|bool Current value
         */
        public function setWrapOutputClass( $className ) {
diff --git a/includes/parser/ParserOutput.php b/includes/parser/ParserOutput.php
index 153a770..fc7b5d9 100644
--- a/includes/parser/ParserOutput.php
+++ b/includes/parser/ParserOutput.php
@@ -23,10 +23,11 @@
  */
 class ParserOutput extends CacheTime {
        /**
-        * Feature flag to indicate to extensions that MediaWiki core supports 
and
+        * Feature flags to indicate to extensions that MediaWiki core supports 
and
         * uses getText() stateless transforms.
         */
        const SUPPORTS_STATELESS_TRANSFORMS = 1;
+       const SUPPORTS_UNWRAP_TRANSFORM = 1;
 
        /**
         * @var string $mText The output text
@@ -266,8 +267,11 @@
         *     to generate one and `__NOTOC__` wasn't used. Default is true,
         *     but might be statefully overridden.
         *  - enableSectionEditLinks: (bool) Include section edit links, 
assuming
-        *    section edit link tokens are present in the HTML. Default is true,
+        *     section edit link tokens are present in the HTML. Default is 
true,
         *     but might be statefully overridden.
+        *  - unwrap: (bool|string) Remove a wrapping mw-parser-output div. If
+        *     passed a string, use that class instead of mw-parser-output. 
Default
+        *     is false.
         * @return string HTML
         */
        public function getText( $options = [] ) {
@@ -284,11 +288,25 @@
                        // In that situation, the historical behavior (possibly 
buggy) is to remove the TOC.
                        'allowTOC' => !empty( $this->mTOCEnabled ),
                        'enableSectionEditLinks' => $this->mEditSectionTokens,
+                       'unwrap' => false,
                ];
                $text = $this->mText;
 
                Hooks::runWithoutAbort( 'ParserOutputPostCacheTransform', [ 
$this, &$text, &$options ] );
 
+               if ( $options['unwrap'] !== false ) {
+                       $start = Html::openElement( 'div', [
+                               'class' => $options['unwrap'] === true ? 
'mw-parser-output' : $options['unwrap']
+                       ] );
+                       $startLen = strlen( $start );
+                       $end = Html::closeElement( 'div' );
+                       $endLen = strlen( $end );
+
+                       if ( substr( $text, 0, $startLen ) === $start && 
substr( $text, -$endLen ) === $end ) {
+                               $text = substr( $text, $startLen, -$endLen );
+                       }
+               }
+
                if ( $options['enableSectionEditLinks'] ) {
                        $text = preg_replace_callback(
                                self::EDITSECTION_REGEX,
diff --git a/tests/parser/ParserTestRunner.php 
b/tests/parser/ParserTestRunner.php
index e07d4a0..9ecdc0d 100644
--- a/tests/parser/ParserTestRunner.php
+++ b/tests/parser/ParserTestRunner.php
@@ -811,10 +811,6 @@
                $options = ParserOptions::newFromContext( $context );
                $options->setTimestamp( $this->getFakeTimestamp() );
 
-               if ( !isset( $opts['wrap'] ) ) {
-                       $options->setWrapOutputClass( false );
-               }
-
                if ( isset( $opts['tidy'] ) ) {
                        if ( !$this->tidySupport->isEnabled() ) {
                                $this->recorder->skipped( $test, 'tidy 
extension is not installed' );
@@ -854,7 +850,8 @@
                } else {
                        $output = $parser->parse( $test['input'], $title, 
$options, true, true, 1337 );
                        $out = $output->getText( [
-                               'allowTOC' => !isset( $opts['notoc'] )
+                               'allowTOC' => !isset( $opts['notoc'] ),
+                               'unwrap' => !isset( $opts['wrap'] ),
                        ] );
                        if ( isset( $opts['tidy'] ) ) {
                                $out = preg_replace( '/\s+$/', '', $out );
diff --git a/tests/phpunit/includes/ExtraParserTest.php 
b/tests/phpunit/includes/ExtraParserTest.php
index aaa135d..75ebd31 100644
--- a/tests/phpunit/includes/ExtraParserTest.php
+++ b/tests/phpunit/includes/ExtraParserTest.php
@@ -26,7 +26,6 @@
                // FIXME: This test should pass without setting global content 
language
                $this->options = ParserOptions::newFromUserAndLang( new User, 
$contLang );
                $this->options->setTemplateCallback( [ __CLASS__, 
'statelessFetchTemplate' ] );
-               $this->options->setWrapOutputClass( false );
                $this->parser = new Parser;
 
                MagicWord::clearCache();
@@ -41,9 +40,8 @@
 
                $title = Title::newFromText( 'Unit test' );
                $options = ParserOptions::newFromUser( new User() );
-               $options->setWrapOutputClass( false );
                $this->assertEquals( "<p>$longLine</p>",
-                       $this->parser->parse( $longLine, $title, $options 
)->getText() );
+                       $this->parser->parse( $longLine, $title, $options 
)->getText( [ 'unwrap' => true ] ) );
        }
 
        /**
@@ -55,7 +53,7 @@
                $parserOutput = $this->parser->parse( "Test\n{{Foo}}\n{{Bar}}", 
$title, $this->options );
                $this->assertEquals(
                        "<p>Test\nContent of <i>Template:Foo</i>\nContent of 
<i>Template:Bar</i>\n</p>",
-                       $parserOutput->getText()
+                       $parserOutput->getText( [ 'unwrap' => true ] )
                );
        }
 
diff --git a/tests/phpunit/includes/parser/ParserOptionsTest.php 
b/tests/phpunit/includes/parser/ParserOptionsTest.php
index ad899bd..f941b8d 100644
--- a/tests/phpunit/includes/parser/ParserOptionsTest.php
+++ b/tests/phpunit/includes/parser/ParserOptionsTest.php
@@ -59,7 +59,7 @@
                        'No overrides' => [ true, [] ],
                        'In-key options are ok' => [ true, [
                                'thumbsize' => 1e100,
-                               'wrapclass' => false,
+                               'printable' => false,
                        ] ],
                        'Non-in-key options are not ok' => [ false, [
                                'removeComments' => false,
@@ -99,7 +99,7 @@
        }
 
        public static function provideOptionsHash() {
-               $used = [ 'wrapclass', 'printable' ];
+               $used = [ 'thumbsize', 'printable' ];
 
                $classWrapper = TestingAccessWrapper::newFromClass( 
ParserOptions::class );
                $classWrapper->getDefaults();
@@ -113,9 +113,9 @@
                        'Canonical options, used some options' => [ $used, 
'canonical', [] ],
                        'Used some options, non-default values' => [
                                $used,
-                               'printable=1!wrapclass=foobar',
+                               'printable=1!thumbsize=200',
                                [
-                                       'wrapclass' => 'foobar',
+                                       'thumbsize' => 200,
                                        'printable' => true,
                                ]
                        ],
diff --git a/tests/phpunit/includes/parser/ParserOutputTest.php 
b/tests/phpunit/includes/parser/ParserOutputTest.php
index 441d60d..6aa9826 100644
--- a/tests/phpunit/includes/parser/ParserOutputTest.php
+++ b/tests/phpunit/includes/parser/ParserOutputTest.php
@@ -125,7 +125,7 @@
        public static function provideGetText() {
                // @codingStandardsIgnoreStart Generic.Files.LineLength
                $text = <<<EOF
-<p>Test document.
+<div class="mw-parser-output"><p>Test document.
 </p>
 <mw:toc><div id="toc" class="toc"><div class="toctitle"><h2>Contents</h2></div>
 <ul>
@@ -150,13 +150,13 @@
 </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>
+</p></div>
 EOF;
 
                return [
                        'No stateless options, default state' => [
                                [], [], $text, <<<EOF
-<p>Test document.
+<div class="mw-parser-output"><p>Test document.
 </p>
 <div id="toc" class="toc"><div class="toctitle"><h2>Contents</h2></div>
 <ul>
@@ -181,12 +181,12 @@
 </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>
+</p></div>
 EOF
                        ],
                        'No stateless options, TOC statefully disabled' => [
                                [], [ 'mTOCEnabled' => false ], $text, <<<EOF
-<p>Test document.
+<div class="mw-parser-output"><p>Test document.
 </p>
 
 <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>
@@ -200,12 +200,12 @@
 </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>
+</p></div>
 EOF
                        ],
                        'No stateless options, section edits statefully 
disabled' => [
                                [], [ 'mEditSectionTokens' => false ], $text, 
<<<EOF
-<p>Test document.
+<div class="mw-parser-output"><p>Test document.
 </p>
 <div id="toc" class="toc"><div class="toctitle"><h2>Contents</h2></div>
 <ul>
@@ -230,14 +230,14 @@
 </p>
 <h2><span class="mw-headline" id="Section_3">Section 3</span></h2>
 <p>Three
-</p>
+</p></div>
 EOF
                        ],
                        'Stateless options override stateful settings' => [
                                [ 'allowTOC' => true, 'enableSectionEditLinks' 
=> true ],
                                [ 'mTOCEnabled' => false, 'mEditSectionTokens' 
=> false ],
                                $text, <<<EOF
-<p>Test document.
+<div class="mw-parser-output"><p>Test document.
 </p>
 <div id="toc" class="toc"><div class="toctitle"><h2>Contents</h2></div>
 <ul>
@@ -262,12 +262,12 @@
 </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>
+</p></div>
 EOF
                        ],
                        'Statelessly disable section edit links' => [
                                [ 'enableSectionEditLinks' => false ], [], 
$text, <<<EOF
-<p>Test document.
+<div class="mw-parser-output"><p>Test document.
 </p>
 <div id="toc" class="toc"><div class="toctitle"><h2>Contents</h2></div>
 <ul>
@@ -292,13 +292,43 @@
 </p>
 <h2><span class="mw-headline" id="Section_3">Section 3</span></h2>
 <p>Three
-</p>
+</p></div>
 EOF
                        ],
                        'Statelessly disable TOC' => [
                                [ 'allowTOC' => false ], [], $text, <<<EOF
+<div class="mw-parser-output"><p>Test document.
+</p>
+
+<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></div>
+EOF
+                       ],
+                       'Statelessly 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
@@ -314,6 +344,12 @@
 </p>
 EOF
                        ],
+                       'Unwrap with custom class name' => [
+                               [ 'unwrap' => 'foobar' ], [], '<div 
class="foobar">Content</div>', 'Content'
+                       ],
+                       'Unwrap with wrong custom class name' => [
+                               [ 'unwrap' => 'foobaz' ], [], '<div 
class="foobar">Content</div>', '<div class="foobar">Content</div>'
+                       ],
                ];
                // @codingStandardsIgnoreEnd
        }
diff --git a/tests/phpunit/includes/parser/TagHooksTest.php 
b/tests/phpunit/includes/parser/TagHooksTest.php
index 7e31cba..2fdaa18 100644
--- a/tests/phpunit/includes/parser/TagHooksTest.php
+++ b/tests/phpunit/includes/parser/TagHooksTest.php
@@ -46,7 +46,6 @@
        private function getParserOptions() {
                global $wgContLang;
                $popt = ParserOptions::newFromUserAndLang( new User, 
$wgContLang );
-               $popt->setWrapOutputClass( false );
                return $popt;
        }
 
@@ -63,7 +62,7 @@
                        Title::newFromText( 'Test' ),
                        $this->getParserOptions()
                );
-               $this->assertEquals( "<p>FooOneBaz\n</p>", 
$parserOutput->getText() );
+               $this->assertEquals( "<p>FooOneBaz\n</p>", 
$parserOutput->getText( [ 'unwrap' => true ] ) );
 
                $parser->mPreprocessor = null; # Break the Parser <-> 
Preprocessor cycle
        }
@@ -98,7 +97,7 @@
                        Title::newFromText( 'Test' ),
                        $this->getParserOptions()
                );
-               $this->assertEquals( "<p>FooOneBaz\n</p>", 
$parserOutput->getText() );
+               $this->assertEquals( "<p>FooOneBaz\n</p>", 
$parserOutput->getText( [ 'unwrap' => true ] ) );
 
                $parser->mPreprocessor = null; # Break the Parser <-> 
Preprocessor cycle
        }

-- 
To view, visit https://gerrit.wikimedia.org/r/399907
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iba16e78c41be992467101e7d83e9c3134765b101
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Anomie <bjor...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to