jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/358238 )
Change subject: ParserOptions: Fix handling of 'editsection'
......................................................................
ParserOptions: Fix handling of 'editsection'
The handling of the 'editsection' option prior to I7fb9ffca9 was
unusual: it was included in the cache key, but the getter didn't ever
flag it as "used". This was overlooked in I7fb9ffca9.
This fixes the handling to restore that behavior. It's no longer
considered to be a real parser option, so changing it won't make
isSafeToCache() fail while reading it won't flag it as 'used'.
But to keep Wikibase working (see T85252), if 'editsection' is supplied
in $forOptions optionsHash() will still include it in the hash so
whatever Wikibase is doing by forcing that doesn't break. The hash when
it is included is the same as was used in I7fb9ffca9 to reuse keys.
Once optionsHashPre30() is removed, Wikibase should be changed to use
some other method to fix T85252 so we can remove that hack from
optionsHash().
Change-Id: I77b5519c5a1122a1fafbfc523b77b2268c0efeb1
---
M includes/parser/ParserOptions.php
M tests/phpunit/includes/parser/ParserOptionsTest.php
2 files changed, 80 insertions(+), 28 deletions(-)
Approvals:
Tim Starling: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/parser/ParserOptions.php
b/includes/parser/ParserOptions.php
index f8ed63f..5be0321 100644
--- a/includes/parser/ParserOptions.php
+++ b/includes/parser/ParserOptions.php
@@ -60,7 +60,6 @@
*/
private static $inCacheKey = [
'dateformat' => true,
- 'editsection' => true,
'numberheadings' => true,
'thumbsize' => true,
'stubthreshold' => true,
@@ -81,6 +80,13 @@
* @note Caching based on parse time is handled externally
*/
private $mTimestamp;
+
+ /**
+ * The edit section flag is in ParserOptions for historical reasons, but
+ * doesn't actually affect the parser output since Feb 2015.
+ * @var bool
+ */
+ private $mEditSection = true;
/**
* Stored user object
@@ -242,23 +248,6 @@
*/
public function setEnableImageWhitelist( $x ) {
return $this->setOptionLegacy( 'enableImageWhitelist', $x );
- }
-
- /**
- * Create "edit section" links?
- * @return bool
- */
- public function getEditSection() {
- return $this->getOption( 'editsection' );
- }
-
- /**
- * Create "edit section" links?
- * @param bool|null $x New value (null is no change)
- * @return bool Old value
- */
- public function setEditSection( $x ) {
- return $this->setOptionLegacy( 'editsection', $x );
}
/**
@@ -879,6 +868,23 @@
}
/**
+ * Create "edit section" links?
+ * @return bool
+ */
+ public function getEditSection() {
+ return $this->mEditSection;
+ }
+
+ /**
+ * Create "edit section" links?
+ * @param bool|null $x New value (null is no change)
+ * @return bool Old value
+ */
+ public function setEditSection( $x ) {
+ return wfSetVar( $this->mEditSection, $x );
+ }
+
+ /**
* Set the redirect target.
*
* Note that setting or changing this does not *make* the page a
redirect
@@ -1041,7 +1047,6 @@
// *UPDATE* ParserOptions::matches() if any of this
changes as needed
self::$defaults = [
'dateformat' => null,
- 'editsection' => true,
'tidy' => false,
'interfaceMessage' => false,
'targetLanguage' => null,
@@ -1256,16 +1261,32 @@
public function optionsHash( $forOptions, $title = null ) {
global $wgRenderHashAppend;
+ $options = $this->options;
+ $defaults = self::getCanonicalOverrides() + self::getDefaults();
+ $inCacheKey = self::$inCacheKey;
+
+ // Historical hack: 'editsection' hasn't been a true parser
option since
+ // Feb 2015 (instead the parser outputs a constant placeholder
and post-parse
+ // processing handles the option). But Wikibase forces it in
$forOptions
+ // and expects the cache key to still vary on it for T85252.
+ // @todo Deprecate and remove this behavior after
optionsHashPre30() is
+ // removed (Wikibase can use addExtraKey() or something
instead).
+ if ( in_array( 'editsection', $forOptions, true ) ) {
+ $options['editsection'] = $this->mEditSection;
+ $defaults['editsection'] = true;
+ $inCacheKey['editsection'] = true;
+ ksort( $inCacheKey );
+ }
+
// We only include used options with non-canonical values in
the key
// so adding a new option doesn't invalidate the entire parser
cache.
// The drawback to this is that changing the default value of
an option
// requires manual invalidation of existing cache entries, as
mentioned
// in the docs on the relevant methods and hooks.
- $defaults = self::getCanonicalOverrides() + self::getDefaults();
$values = [];
- foreach ( self::$inCacheKey as $option => $include ) {
+ foreach ( $inCacheKey as $option => $include ) {
if ( $include && in_array( $option, $forOptions, true )
) {
- $v = $this->optionToString(
$this->options[$option] );
+ $v = $this->optionToString( $options[$option] );
$d = $this->optionToString( $defaults[$option]
);
if ( $v !== $d ) {
$values[] = "$option=$v";
@@ -1364,7 +1385,7 @@
// directly. At least Wikibase does at this point in time.
if ( !in_array( 'editsection', $forOptions ) ) {
$confstr .= '!*';
- } elseif ( !$this->options['editsection'] ) {
+ } elseif ( !$this->mEditSection ) {
$confstr .= '!edit=0';
}
diff --git a/tests/phpunit/includes/parser/ParserOptionsTest.php
b/tests/phpunit/includes/parser/ParserOptionsTest.php
index 81f0564..d606142 100644
--- a/tests/phpunit/includes/parser/ParserOptionsTest.php
+++ b/tests/phpunit/includes/parser/ParserOptionsTest.php
@@ -22,7 +22,6 @@
return [
'No overrides' => [ true, [] ],
'In-key options are ok' => [ true, [
- 'editsection' => false,
'thumbsize' => 1e100,
'wrapclass' => false,
] ],
@@ -65,14 +64,14 @@
}
public static function provideOptionsHashPre30() {
- $used = [ 'wrapclass', 'editsection', 'printable' ];
+ $used = [ 'wrapclass', 'printable' ];
return [
'Canonical options, nothing used' => [ [],
'*!*!*!*!*!*', [] ],
- 'Canonical options, used some options' => [ $used,
'*!*!*!*!*', [] ],
+ 'Canonical options, used some options' => [ $used,
'*!*!*!*!*!*', [] ],
'Used some options, non-default values' => [
$used,
- '*!*!*!*!*!printable=1!wrapclass=foobar',
+ '*!*!*!*!*!*!printable=1!wrapclass=foobar',
[
'setWrapOutputClass' => 'foobar',
'setIsPrintable' => true,
@@ -87,6 +86,14 @@
'wgHooks' => [ 'PageRenderingHash' => [
[ __CLASS__ . '::onPageRenderingHash' ] ] ],
]
],
+
+ // Test weird historical behavior is still weird
+ 'Canonical options, editsection=true used' => [ [
'editsection' ], '*!*!*!*!*', [
+ 'setEditSection' => true,
+ ] ],
+ 'Canonical options, editsection=false used' => [ [
'editsection' ], '*!*!*!*!*!edit=0', [
+ 'setEditSection' => false,
+ ] ],
];
}
@@ -117,7 +124,7 @@
}
public static function provideOptionsHash() {
- $used = [ 'wrapclass', 'editsection', 'printable' ];
+ $used = [ 'wrapclass', 'printable' ];
$classWrapper = TestingAccessWrapper::newFromClass(
ParserOptions::class );
$classWrapper->getDefaults();
@@ -154,6 +161,30 @@
$confstr .= '!onPageRenderingHash';
}
+ // Test weird historical behavior is still weird
+ public function testOptionsHashEditSection() {
+ global $wgHooks;
+
+ $this->setMwGlobals( [
+ 'wgRenderHashAppend' => '',
+ 'wgHooks' => [ 'PageRenderingHash' => [] ] + $wgHooks,
+ ] );
+
+ $popt = ParserOptions::newCanonical();
+ $popt->registerWatcher( function ( $name ) {
+ $this->assertNotEquals( 'editsection', $name );
+ } );
+
+ $this->assertTrue( $popt->getEditSection() );
+ $this->assertSame( 'canonical', $popt->optionsHash( [] ) );
+ $this->assertSame( 'canonical', $popt->optionsHash( [
'editsection' ] ) );
+
+ $popt->setEditSection( false );
+ $this->assertFalse( $popt->getEditSection() );
+ $this->assertSame( 'canonical', $popt->optionsHash( [] ) );
+ $this->assertSame( 'editsection=0', $popt->optionsHash( [
'editsection' ] ) );
+ }
+
/**
* @expectedException InvalidArgumentException
* @expectedExceptionMessage Unknown parser option bogus
--
To view, visit https://gerrit.wikimedia.org/r/358238
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I77b5519c5a1122a1fafbfc523b77b2268c0efeb1
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Anomie <bjor...@wikimedia.org>
Gerrit-Reviewer: C. Scott Ananian <canan...@wikimedia.org>
Gerrit-Reviewer: Jackmcbarn <jackmcb...@gmail.com>
Gerrit-Reviewer: Legoktm <lego...@member.fsf.org>
Gerrit-Reviewer: Tim Starling <tstarl...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
