jenkins-bot has submitted this change and it was merged.
Change subject: Revert "More sensible behavior when special page aliases
conflict"
......................................................................
Revert "More sensible behavior when special page aliases conflict"
Causes context's title to be null in some cases
This reverts commit ad522beeea5037ced24781df515b880cf75733ca.
Change-Id: I443282ef4ef6212f1e533f34dea7335b2282a718
---
M includes/specialpage/SpecialPageFactory.php
M languages/Language.php
M tests/phpunit/includes/specialpage/SpecialPageFactoryTest.php
3 files changed, 36 insertions(+), 197 deletions(-)
Approvals:
Reedy: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/specialpage/SpecialPageFactory.php
b/includes/specialpage/SpecialPageFactory.php
index 7904140..48bcb77 100644
--- a/includes/specialpage/SpecialPageFactory.php
+++ b/includes/specialpage/SpecialPageFactory.php
@@ -262,9 +262,10 @@
/**
* Initialise and return the list of special page aliases. Returns an
object with
- * properties which can be accessed $obj->pagename - each property name
is an
- * alias, with the value being the canonical name of the special page.
All
- * registered special pages are guaranteed to map to themselves.
+ * properties which can be accessed $obj->pagename - each property is
an array of
+ * aliases; the first in the array is the canonical alias. All
registered special
+ * pages are guaranteed to have a property entry, and for that property
array to
+ * contain at least one entry (English fallbacks will be added if
necessary).
* @return object
*/
private static function getAliasListObject() {
@@ -272,43 +273,20 @@
global $wgContLang;
$aliases = $wgContLang->getSpecialPageAliases();
+ $missingPages = self::getPageList();
+
self::$aliases = array();
- $keepAlias = array();
-
- // Force every canonical name to be an alias for itself.
- foreach ( self::getPageList() as $name => $stuff ) {
- $caseFoldedAlias = $wgContLang->caseFold( $name
);
- self::$aliases[$caseFoldedAlias] = $name;
- $keepAlias[$caseFoldedAlias] = 'canonical';
- }
-
// Check for $aliases being an array since
Language::getSpecialPageAliases can return null
if ( is_array( $aliases ) ) {
foreach ( $aliases as $realName => $aliasList )
{
- $aliasList = array_values( $aliasList );
- foreach ( $aliasList as $i => $alias ) {
- $caseFoldedAlias =
$wgContLang->caseFold( $alias );
-
- if ( isset(
self::$aliases[$caseFoldedAlias] ) &&
- $realName ===
self::$aliases[$caseFoldedAlias]
- ) {
- // Ignore same-realName
conflicts
- continue;
- }
-
- if ( !isset(
$keepAlias[$caseFoldedAlias] ) ) {
-
self::$aliases[$caseFoldedAlias] = $realName;
- if ( !$i ) {
-
$keepAlias[$caseFoldedAlias] = 'first';
- }
- } elseif ( !$i ) {
- wfWarn( "First alias
'$alias' for $realName conflicts with " .
-
"{$keepAlias[$caseFoldedAlias]} alias for " .
-
self::$aliases[$caseFoldedAlias]
- );
- }
+ foreach ( $aliasList as $alias ) {
+
self::$aliases[$wgContLang->caseFold( $alias )] = $realName;
}
+ unset( $missingPages->$realName );
}
+ }
+ foreach ( $missingPages as $name => $stuff ) {
+ self::$aliases[$wgContLang->caseFold( $name )]
= $name;
}
// Cast to object: func()[$key] doesn't work, but
func()->$key does
@@ -642,42 +620,29 @@
public static function getLocalNameFor( $name, $subpage = false ) {
global $wgContLang;
$aliases = $wgContLang->getSpecialPageAliases();
- $aliasList = self::getAliasListObject();
- // Find the first alias that maps back to $name
- if ( isset( $aliases[$name] ) ) {
- $found = false;
- foreach ( $aliases[$name] as $alias ) {
- $caseFoldedAlias = $wgContLang->caseFold(
$alias );
- $caseFoldedAlias = str_replace( ' ', '_',
$caseFoldedAlias );
- if ( isset( $aliasList->$caseFoldedAlias ) &&
- $aliasList->$caseFoldedAlias === $name
- ) {
- $name = $alias;
- $found = true;
- break;
- }
- }
- if ( !$found ) {
- wfWarn( "Did not find a usable alias for
special page '$name'. " .
- "It seems all defined aliases
conflict?" );
- }
+ if ( isset( $aliases[$name][0] ) ) {
+ $name = $aliases[$name][0];
} else {
- // Check if someone misspelled the correct casing
+ // Try harder in case someone misspelled the correct
casing
+ $found = false;
+ // Check for $aliases being an array since
Language::getSpecialPageAliases can return null
if ( is_array( $aliases ) ) {
foreach ( $aliases as $n => $values ) {
if ( strcasecmp( $name, $n ) === 0 ) {
wfWarn( "Found alias defined
for $n when searching for " .
"special page aliases
for $name. Case mismatch?" );
- return self::getLocalNameFor(
$n, $subpage );
+ $name = $values[0];
+ $found = true;
+ break;
}
}
}
-
- wfWarn( "Did not find alias for special page '$name'. "
.
- "Perhaps no aliases are defined for it?" );
+ if ( !$found ) {
+ wfWarn( "Did not find alias for special page
'$name'. " .
+ "Perhaps no aliases are defined for
it?" );
+ }
}
-
if ( $subpage !== false && !is_null( $subpage ) ) {
$name = "$name/$subpage";
}
diff --git a/languages/Language.php b/languages/Language.php
index b985077..bf2e3a3 100644
--- a/languages/Language.php
+++ b/languages/Language.php
@@ -737,8 +737,6 @@
}
/**
- * @deprecated since 1.24, doesn't handle conflicting aliases. Use
- * SpecialPageFactory::getLocalNameFor instead.
* @param string $name
* @return string
*/
diff --git a/tests/phpunit/includes/specialpage/SpecialPageFactoryTest.php
b/tests/phpunit/includes/specialpage/SpecialPageFactoryTest.php
index 4619c2e..d56ecad 100644
--- a/tests/phpunit/includes/specialpage/SpecialPageFactoryTest.php
+++ b/tests/phpunit/includes/specialpage/SpecialPageFactoryTest.php
@@ -22,12 +22,6 @@
*/
class SpecialPageFactoryTest extends MediaWikiTestCase {
- protected function tearDown() {
- parent::tearDown();
-
- SpecialPageFactory::resetList();
- }
-
public function newSpecialAllPages() {
return new SpecialAllPages();
}
@@ -48,6 +42,7 @@
*/
public function testGetPage( $spec, $shouldReuseInstance ) {
$this->mergeMwGlobalArrayValue( 'wgSpecialPages', array(
'testdummy' => $spec ) );
+
SpecialPageFactory::resetList();
$page = SpecialPageFactory::getPage( 'testdummy' );
@@ -55,6 +50,8 @@
$page2 = SpecialPageFactory::getPage( 'testdummy' );
$this->assertEquals( $shouldReuseInstance, $page2 === $page,
"Should re-use instance:" );
+
+ SpecialPageFactory::resetList();
}
/**
@@ -62,11 +59,12 @@
*/
public function testGetNames() {
$this->mergeMwGlobalArrayValue( 'wgSpecialPages', array(
'testdummy' => 'SpecialAllPages' ) );
- SpecialPageFactory::resetList();
+ SpecialPageFactory::resetList();
$names = SpecialPageFactory::getNames();
$this->assertInternalType( 'array', $names );
$this->assertContains( 'testdummy', $names );
+ SpecialPageFactory::resetList();
}
/**
@@ -74,11 +72,14 @@
*/
public function testResolveAlias() {
$this->setMwGlobals( 'wgContLang', Language::factory( 'de' ) );
+
SpecialPageFactory::resetList();
list( $name, $param ) = SpecialPageFactory::resolveAlias(
'Spezialseiten/Foo' );
$this->assertEquals( 'Specialpages', $name );
$this->assertEquals( 'Foo', $param );
+
+ SpecialPageFactory::resetList();
}
/**
@@ -86,10 +87,13 @@
*/
public function testGetLocalNameFor() {
$this->setMwGlobals( 'wgContLang', Language::factory( 'de' ) );
+
SpecialPageFactory::resetList();
$name = SpecialPageFactory::getLocalNameFor( 'Specialpages',
'Foo' );
$this->assertEquals( 'Spezialseiten/Foo', $name );
+
+ SpecialPageFactory::resetList();
}
/**
@@ -97,142 +101,14 @@
*/
public function testGetTitleForAlias() {
$this->setMwGlobals( 'wgContLang', Language::factory( 'de' ) );
+
SpecialPageFactory::resetList();
$title = SpecialPageFactory::getTitleForAlias(
'Specialpages/Foo' );
$this->assertEquals( 'Spezialseiten/Foo', $title->getText() );
$this->assertEquals( NS_SPECIAL, $title->getNamespace() );
- }
- /**
- * @dataProvider provideTestConflictResolution
- */
- public function testConflictResolution(
- $test, $aliasesList, $alias, $expectedName, $expectedAlias,
$expectWarnings
- ) {
- global $wgContLang;
- $lang = clone $wgContLang;
- $lang->mExtendedSpecialPageAliases = $aliasesList;
- $this->setMwGlobals( 'wgContLang', $lang );
- $this->setMwGlobals( 'wgSpecialPages',
- array_combine( array_keys( $aliasesList ), array_keys(
$aliasesList ) )
- );
SpecialPageFactory::resetList();
-
- // Catch the warnings we expect to be raised
- $warnings = array();
- $this->setMwGlobals( 'wgDevelopmentWarnings', true );
- set_error_handler( function ( $errno, $errstr ) use (
&$warnings ) {
- if ( preg_match( '/First alias \'[^\']*\' for .*/',
$errstr ) ||
- preg_match( '/Did not find a usable alias for
special page .*/', $errstr )
- ) {
- $warnings[] = $errstr;
- return true;
- }
- return false;
- } );
- $reset = new ScopedCallback( 'restore_error_handler' );
-
- list( $name, /*...*/ ) = SpecialPageFactory::resolveAlias(
$alias );
- $this->assertEquals( $expectedName, $name, "$test: Alias to
name" );
- $result = SpecialPageFactory::getLocalNameFor( $name );
- $this->assertEquals( $expectedAlias, $result, "$test: Alias to
name to alias" );
-
- $gotWarnings = count( $warnings );
- if ( $gotWarnings !== $expectWarnings ) {
- $this->fail( "Expected $expectWarnings warning(s), but
got $gotWarnings:\n" .
- join( "\n", $warnings )
- );
- }
- }
-
- /**
- * @dataProvider provideTestConflictResolution
- */
- public function testConflictResolutionReversed(
- $test, $aliasesList, $alias, $expectedName, $expectedAlias,
$expectWarnings
- ) {
- // Make sure order doesn't matter by reversing the list
- $aliasesList = array_reverse( $aliasesList );
- return $this->testConflictResolution(
- $test, $aliasesList, $alias, $expectedName,
$expectedAlias, $expectWarnings
- );
- }
-
- public function provideTestConflictResolution() {
- return array(
- array(
- 'Canonical name wins',
- array( 'Foo' => array( 'Foo', 'Bar' ), 'Baz' =>
array( 'Foo', 'BazPage', 'Baz2' ) ),
- 'Foo',
- 'Foo',
- 'Foo',
- 1,
- ),
-
- array(
- 'Doesn\'t redirect to a different special
page\'s canonical name',
- array( 'Foo' => array( 'Foo', 'Bar' ), 'Baz' =>
array( 'Foo', 'BazPage', 'Baz2' ) ),
- 'Baz',
- 'Baz',
- 'BazPage',
- 1,
- ),
-
- array(
- 'Canonical name wins even if not aliased',
- array( 'Foo' => array( 'FooPage' ), 'Baz' =>
array( 'Foo', 'BazPage', 'Baz2' ) ),
- 'Foo',
- 'Foo',
- 'FooPage',
- 1,
- ),
-
- array(
- 'Doesn\'t redirect to a different special
page\'s canonical name even if not aliased',
- array( 'Foo' => array( 'FooPage' ), 'Baz' =>
array( 'Foo', 'BazPage', 'Baz2' ) ),
- 'Baz',
- 'Baz',
- 'BazPage',
- 1,
- ),
-
- array(
- 'First local name beats non-first',
- array( 'First' => array( 'Foo' ), 'NonFirst' =>
array( 'Bar', 'Foo' ) ),
- 'Foo',
- 'First',
- 'Foo',
- 0,
- ),
-
- array(
- 'Doesn\'t redirect to a different special
page\'s first alias',
- array(
- 'Foo' => array( 'Foo' ),
- 'First' => array( 'Bar' ),
- 'Baz' => array( 'Foo', 'Bar',
'BazPage', 'Baz2' )
- ),
- 'Baz',
- 'Baz',
- 'BazPage',
- 1,
- ),
-
- array(
- 'Doesn\'t redirect wrong even if all aliases
conflict',
- array(
- 'Foo' => array( 'Foo' ),
- 'First' => array( 'Bar' ),
- 'Baz' => array( 'Foo', 'Bar' )
- ),
- 'Baz',
- 'Baz',
- 'Baz',
- 2,
- ),
-
- );
}
}
--
To view, visit https://gerrit.wikimedia.org/r/162957
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I443282ef4ef6212f1e533f34dea7335b2282a718
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/core
Gerrit-Branch: wmf/1.25wmf1
Gerrit-Owner: Reedy <re...@wikimedia.org>
Gerrit-Reviewer: MaxSem <maxsem.w...@gmail.com>
Gerrit-Reviewer: Reedy <re...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
