Skip to content

Commit d511626

Browse files
anomiextgr
anomiex
authored and
tgr
committed
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
1 parent 58bc334 commit d511626

File tree

12 files changed

+89
-41
lines changed

12 files changed

+89
-41
lines changed

‎RELEASE-NOTES-1.31‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,8 @@ changes to languages because of Phabricator reports.
194194
Setting template variables by reference allowed violating the principle of data being
195195
immutable once added to the skin template. In practice, this method was not being
196196
used for that. Rather, setRef() existed as memory optimisation for PHP 4.
197+
* Passing false to ParserOptions::setWrapOutputClass() is deprecated. Use the
198+
'unwrap' transform to ParserOutput::getText() instead.
197199

198200
== Compatibility ==
199201
MediaWiki 1.31 requires PHP 5.5.9 or later. Although HHVM 3.18.5 or later is supported,

‎includes/Message.php‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1245,7 +1245,14 @@ protected function parseText( $string ) {
12451245
);
12461246

12471247
return $out instanceof ParserOutput
1248-
? $out->getText( [ 'enableSectionEditLinks' => false ] )
1248+
? $out->getText( [
1249+
'enableSectionEditLinks' => false,
1250+
// Wrapping messages in an extra <div> is probably not expected. If
1251+
// they're outside the content area they probably shouldn't be
1252+
// targeted by CSS that's targeting the parser output, and if
1253+
// they're inside they already are from the outer div.
1254+
'unwrap' => true,
1255+
] )
12491256
: $out;
12501257
}
12511258

‎includes/api/ApiParse.php‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,7 @@ public function execute() {
344344
$result_array['text'] = $p_result->getText( [
345345
'allowTOC' => !$params['disabletoc'],
346346
'enableSectionEditLinks' => !$params['disableeditsection'],
347+
'unwrap' => $params['wrapoutputclass'] === '',
347348
] );
348349
$result_array[ApiResult::META_BC_SUBELEMENTS][] = 'text';
349350
}
@@ -538,9 +539,9 @@ protected function makeParserOptions( WikiPage $pageObj, array $params ) {
538539
if ( $params['disabletidy'] ) {
539540
$popts->setTidy( false );
540541
}
541-
$popts->setWrapOutputClass(
542-
$params['wrapoutputclass'] === '' ? false : $params['wrapoutputclass']
543-
);
542+
if ( $params['wrapoutputclass'] !== '' ) {
543+
$popts->setWrapOutputClass( $params['wrapoutputclass'] );
544+
}
544545

545546
$reset = null;
546547
$suppressCache = false;

‎includes/cache/MessageCache.php‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,6 @@ function getParserOptions() {
193193
$po = ParserOptions::newFromAnon();
194194
$po->setEditSection( false );
195195
$po->setAllowUnsafeRawHtml( false );
196-
$po->setWrapOutputClass( false );
197196
return $po;
198197
}
199198

@@ -203,11 +202,6 @@ function getParserOptions() {
203202
// from malicious sources. As a precaution, disable
204203
// the <html> parser tag when parsing messages.
205204
$this->mParserOptions->setAllowUnsafeRawHtml( false );
206-
// Wrapping messages in an extra <div> is probably not expected. If
207-
// they're outside the content area they probably shouldn't be
208-
// targeted by CSS that's targeting the parser output, and if
209-
// they're inside they already are from the outer div.
210-
$this->mParserOptions->setWrapOutputClass( false );
211205
}
212206

213207
return $this->mParserOptions;

‎includes/installer/Installer.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -447,7 +447,6 @@ public function __construct() {
447447
$this->parserTitle = Title::newFromText( 'Installer' );
448448
$this->parserOptions = new ParserOptions( $wgUser ); // language will be wrong :(
449449
$this->parserOptions->setEditSection( false );
450-
$this->parserOptions->setWrapOutputClass( false );
451450
// Don't try to access DB before user language is initialised
452451
$this->setParserLanguage( Language::factory( 'en' ) );
453452
}
@@ -689,6 +688,7 @@ public function parse( $text, $lineStart = false ) {
689688
$out = $wgParser->parse( $text, $this->parserTitle, $this->parserOptions, $lineStart );
690689
$html = $out->getText( [
691690
'enableSectionEditLinks' => false,
691+
'unwrap' => true,
692692
] );
693693
} catch ( MediaWiki\Services\ServiceDisabledException $e ) {
694694
$html = '<!--DB access attempted during parse--> ' . htmlspecialchars( $text );

‎includes/parser/ParserOptions.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -781,6 +781,7 @@ public function getWrapOutputClass() {
781781
* CSS class to use to wrap output from Parser::parse()
782782
* @since 1.30
783783
* @param string|bool $className Set false to disable wrapping.
784+
* Passing false is deprecated since MediaWiki 1.31
784785
* @return string|bool Current value
785786
*/
786787
public function setWrapOutputClass( $className ) {

‎includes/parser/ParserOutput.php‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,11 @@
2323
*/
2424
class ParserOutput extends CacheTime {
2525
/**
26-
* Feature flag to indicate to extensions that MediaWiki core supports and
26+
* Feature flags to indicate to extensions that MediaWiki core supports and
2727
* uses getText() stateless transforms.
2828
*/
2929
const SUPPORTS_STATELESS_TRANSFORMS = 1;
30+
const SUPPORTS_UNWRAP_TRANSFORM = 1;
3031

3132
/**
3233
* @var string $mText The output text
@@ -266,8 +267,9 @@ public function getRawText() {
266267
* to generate one and `__NOTOC__` wasn't used. Default is true,
267268
* but might be statefully overridden.
268269
* - enableSectionEditLinks: (bool) Include section edit links, assuming
269-
* section edit link tokens are present in the HTML. Default is true,
270+
* section edit link tokens are present in the HTML. Default is true,
270271
* but might be statefully overridden.
272+
* - unwrap: (bool) Remove a wrapping mw-parser-output div. Default is false.
271273
* @return string HTML
272274
*/
273275
public function getText( $options = [] ) {
@@ -284,11 +286,25 @@ public function getText( $options = [] ) {
284286
// In that situation, the historical behavior (possibly buggy) is to remove the TOC.
285287
'allowTOC' => !empty( $this->mTOCEnabled ),
286288
'enableSectionEditLinks' => $this->mEditSectionTokens,
289+
'unwrap' => false,
287290
];
288291
$text = $this->mText;
289292

290293
Hooks::runWithoutAbort( 'ParserOutputPostCacheTransform', [ $this, &$text, &$options ] );
291294

295+
if ( $options['unwrap'] !== false ) {
296+
$start = Html::openElement( 'div', [
297+
'class' => 'mw-parser-output'
298+
] );
299+
$startLen = strlen( $start );
300+
$end = Html::closeElement( 'div' );
301+
$endLen = strlen( $end );
302+
303+
if ( substr( $text, 0, $startLen ) === $start && substr( $text, -$endLen ) === $end ) {
304+
$text = substr( $text, $startLen, -$endLen );
305+
}
306+
}
307+
292308
if ( $options['enableSectionEditLinks'] ) {
293309
$text = preg_replace_callback(
294310
self::EDITSECTION_REGEX,

‎tests/parser/ParserTestRunner.php‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -811,10 +811,6 @@ public function runTest( $test ) {
811811
$options = ParserOptions::newFromContext( $context );
812812
$options->setTimestamp( $this->getFakeTimestamp() );
813813

814-
if ( !isset( $opts['wrap'] ) ) {
815-
$options->setWrapOutputClass( false );
816-
}
817-
818814
if ( isset( $opts['tidy'] ) ) {
819815
if ( !$this->tidySupport->isEnabled() ) {
820816
$this->recorder->skipped( $test, 'tidy extension is not installed' );
@@ -854,7 +850,8 @@ public function runTest( $test ) {
854850
} else {
855851
$output = $parser->parse( $test['input'], $title, $options, true, true, 1337 );
856852
$out = $output->getText( [
857-
'allowTOC' => !isset( $opts['notoc'] )
853+
'allowTOC' => !isset( $opts['notoc'] ),
854+
'unwrap' => !isset( $opts['wrap'] ),
858855
] );
859856
if ( isset( $opts['tidy'] ) ) {
860857
$out = preg_replace( '/\s+$/', '', $out );

‎tests/phpunit/includes/ExtraParserTest.php‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ protected function setUp() {
2626
// FIXME: This test should pass without setting global content language
2727
$this->options = ParserOptions::newFromUserAndLang( new User, $contLang );
2828
$this->options->setTemplateCallback( [ __CLASS__, 'statelessFetchTemplate' ] );
29-
$this->options->setWrapOutputClass( false );
3029
$this->parser = new Parser;
3130

3231
MagicWord::clearCache();
@@ -41,9 +40,8 @@ public function testLongNumericLinesDontKillTheParser() {
4140

4241
$title = Title::newFromText( 'Unit test' );
4342
$options = ParserOptions::newFromUser( new User() );
44-
$options->setWrapOutputClass( false );
4543
$this->assertEquals( "<p>$longLine</p>",
46-
$this->parser->parse( $longLine, $title, $options )->getText() );
44+
$this->parser->parse( $longLine, $title, $options )->getText( [ 'unwrap' => true ] ) );
4745
}
4846

4947
/**
@@ -55,7 +53,7 @@ public function testParse() {
5553
$parserOutput = $this->parser->parse( "Test\n{{Foo}}\n{{Bar}}", $title, $this->options );
5654
$this->assertEquals(
5755
"<p>Test\nContent of <i>Template:Foo</i>\nContent of <i>Template:Bar</i>\n</p>",
58-
$parserOutput->getText()
56+
$parserOutput->getText( [ 'unwrap' => true ] )
5957
);
6058
}
6159

‎tests/phpunit/includes/parser/ParserOptionsTest.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public static function provideIsSafeToCache() {
6262
'No overrides' => [ true, [] ],
6363
'In-key options are ok' => [ true, [
6464
'thumbsize' => 1e100,
65-
'wrapclass' => false,
65+
'printable' => false,
6666
] ],
6767
'Non-in-key options are not ok' => [ false, [
6868
'removeComments' => false,
@@ -102,7 +102,7 @@ public function testOptionsHash( $usedOptions, $expect, $options, $globals = []
102102
}
103103

104104
public static function provideOptionsHash() {
105-
$used = [ 'wrapclass', 'printable' ];
105+
$used = [ 'thumbsize', 'printable' ];
106106

107107
$classWrapper = TestingAccessWrapper::newFromClass( ParserOptions::class );
108108
$classWrapper->getDefaults();
@@ -116,9 +116,9 @@ public static function provideOptionsHash() {
116116
'Canonical options, used some options' => [ $used, 'canonical', [] ],
117117
'Used some options, non-default values' => [
118118
$used,
119-
'printable=1!wrapclass=foobar',
119+
'printable=1!thumbsize=200',
120120
[
121-
'wrapclass' => 'foobar',
121+
'thumbsize' => 200,
122122
'printable' => true,
123123
]
124124
],

0 commit comments

Comments
 (0)