From 0de054cd7963c84c74cac6e284633bbf517e40de Mon Sep 17 00:00:00 2001 From: Piotr Miazga Date: Thu, 24 Aug 2017 22:51:39 +0200 Subject: [PATCH] Use canonical name for NS_SPECIAL titles when checking the blacklist Changes - when verifying title use canonical names for pages in special namespace - improve unit tests to verify canonical names and translated titles Bug: T170169 Change-Id: I49592133eb8286eacf04fd3034df091f7ef2aa50 --- extension.json | 4 ++-- includes/PopupsContext.php | 14 +++++++++++++- tests/phpunit/PopupsContextTest.php | 24 +++++++++++++++++++++--- 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/extension.json b/extension.json index 0bbf610ed..501aef35c 100644 --- a/extension.json +++ b/extension.json @@ -72,8 +72,8 @@ "PopupsEventLogging": false, "@PopupsStatsvSamplingRate": "Sampling rate for logging performance data to statsv.", "PopupsStatsvSamplingRate": 0, - "@PopupsPageBlacklist": "Blacklisted pages are subject to the HTML cache policy of the wiki. A purge on a blacklisted page maybe needed to see the effect of this configuration variable.", - "PopupsPageBlacklist": [ "Special:UserLogin", "Special:CreateAccount" ] + "@PopupsPageBlacklist": "Blacklisted pages are subject to the HTML cache policy of the wiki. A purge on a blacklisted page maybe needed to see the effect of this configuration variable. Every blacklisted page should be defined by a canonical name, eg: Special:Userlogin", + "PopupsPageBlacklist": [ "Special:Userlogin", "Special:CreateAccount" ] }, "ResourceModules": { "ext.popups.images": { diff --git a/includes/PopupsContext.php b/includes/PopupsContext.php index 8b3bdfa9c..2a3e09054 100644 --- a/includes/PopupsContext.php +++ b/includes/PopupsContext.php @@ -177,9 +177,21 @@ class PopupsContext { */ public function isTitleBlacklisted( $title ) { $blacklistedPages = $this->config->get( 'PopupsPageBlacklist' ); + $canonicalTitle = $title->getRootTitle(); + + if ( $title->isSpecialPage() ) { + // it's special page, translate it to canonical name + list( $name, $subpage ) = \SpecialPageFactory::resolveAlias( $canonicalTitle->getText() ); + + if ( $name !== null ) { + $canonicalTitle = Title::newFromText( $name, NS_SPECIAL ); + } + } + foreach ( $blacklistedPages as $page ) { $blacklistedTitle = Title::newFromText( $page ); - if ( $title->getRootTitle() == $blacklistedTitle->getRootTitle() ) { + + if ( $canonicalTitle->equals( $blacklistedTitle ) ) { return true; } } diff --git a/tests/phpunit/PopupsContextTest.php b/tests/phpunit/PopupsContextTest.php index 2976b7c3e..67bbd9ead 100644 --- a/tests/phpunit/PopupsContextTest.php +++ b/tests/phpunit/PopupsContextTest.php @@ -279,21 +279,39 @@ class PopupsContextTest extends MediaWikiTestCase { } /** - * @return array/ + * @return array */ public function provideTestIsTitleBlacklisted() { - $blacklist = [ 'Special:UserLogin', 'Special:CreateAccount', 'User:A' ]; + $blacklist = [ 'Special:Userlogin', 'Special:CreateAccount', 'User:A' ]; return [ [ $blacklist, Title::newFromText( 'Main_Page' ), false ], - [ $blacklist, Title::newFromText( 'Special:UserLogin' ), true ], [ $blacklist, Title::newFromText( 'Special:CreateAccount' ), true ], [ $blacklist, Title::newFromText( 'User:A' ), true ], [ $blacklist, Title::newFromText( 'User:A/B' ), true ], [ $blacklist, Title::newFromText( 'User:B' ), false ], [ $blacklist, Title::newFromText( 'User:B/A' ), false ], + // test canonical name handling + [ $blacklist, Title::newFromText( 'Special:UserLogin' ), true ], ]; } + /** + * Test if special page in different language is blacklisted + * + * @covers ::isTitleBlacklisted + */ + public function testIsTranslatedTitleBlacklisted() { + $page = 'Specjalna:Preferencje'; + $blacklist = [ $page ]; + + $this->setMwGlobals( [ + "wgPopupsPageBlacklist" => $blacklist, + "wgLanguageCode" => "pl" + ] ); + $context = $this->getContext(); + $this->assertEquals( true, $context->isTitleBlacklisted( Title::newFromText( $page ) ) ); + } + /** * @covers ::getDefaultIsEnabledState */