[MediaWiki-commits] [Gerrit] mediawiki...Popups[master]: Use canonical name for NS_SPECIAL titles when checking the b...

2017-08-25 Thread jenkins-bot (Code Review) 
jenkins-bot has submitted this change and it was merged. ( 
https://gerrit.wikimedia.org/r/373688 )

Change subject: Use canonical name for NS_SPECIAL titles when checking the 
blacklist
..


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
---
M extension.json
M includes/PopupsContext.php
M tests/phpunit/PopupsContextTest.php
3 files changed, 36 insertions(+), 6 deletions(-)

Approvals:
  jenkins-bot: Verified
  Jdlrobson: Looks good to me, approved



diff --git a/extension.json b/extension.json
index 0bbf610..501aef3 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 8b3bdfa..2a3e090 100644
--- a/includes/PopupsContext.php
+++ b/includes/PopupsContext.php
@@ -177,9 +177,21 @@
 */
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 2976b7c..67bbd9e 100644
--- a/tests/phpunit/PopupsContextTest.php
+++ b/tests/phpunit/PopupsContextTest.php
@@ -279,22 +279,40 @@
}
 
/**
-* @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(

[MediaWiki-commits] [Gerrit] mediawiki...Popups[master]: Use canonical name for NS_SPECIAL titles when checking the b...

2017-08-24 Thread Pmiazga (Code Review) 
Pmiazga has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/373688 )

Change subject: Use canonical name for NS_SPECIAL titles when checking the 
blacklist
..

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: T173597
Change-Id: I49592133eb8286eacf04fd3034df091f7ef2aa50
---
M includes/PopupsContext.php
M tests/phpunit/PopupsContextTest.php
2 files changed, 21 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Popups 
refs/changes/88/373688/1

diff --git a/includes/PopupsContext.php b/includes/PopupsContext.php
index 8b3bdfa..2a3e090 100644
--- a/includes/PopupsContext.php
+++ b/includes/PopupsContext.php
@@ -177,9 +177,21 @@
 */
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 2976b7c..4534034 100644
--- a/tests/phpunit/PopupsContextTest.php
+++ b/tests/phpunit/PopupsContextTest.php
@@ -272,25 +272,29 @@
 * @param Title $title
 $ @param bool $expected
 */
-   public function testIsTitleBlacklisted( $blacklist, Title $title, 
$expected ) {
+   public function testIsTitleBlacklisted( array $blacklist, Title $title, 
$expected ) {
$this->setMwGlobals( [ "wgPopupsPageBlacklist" => $blacklist ] 
);
$context = $this->getContext();
-   $this->assertEquals( $expected, $context->isTitleBlacklisted( 
$title ) );
+   $this->assertEquals( $expected, $context->isTitleBlacklisted( 
$title ),
+   'isBlacklisted check failed for ' + 
$title->getPrefixedText() );
}
 
/**
 * @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 ],
+   // handle different languages
+   [ $blacklist, Title::newFromText( 'Specjalna:Zaloguj' 
), true ],
];
}
 

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I49592133eb8286eacf04fd3034df091f7ef2aa50
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Popups
Gerrit-Branch: master
Gerrit-Owner: Pmiazga 

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