Remove is_numeric check from Title::checkUserBlock

This should allow the usernames of administrators such as "7"
to show correctly on permissions error pages.

I extracted the working code from UserBlockedError::__construct
into a separate method Block::getPermissionsError, called from
both places with context provided as an argument.

Additional changes to get the test suite to pass are included.

Bug: 46768
Change-Id: I49d973992a99e03b4e8de112b47b737037a85338

diff --git a/RELEASE-NOTES-1.22 b/RELEASE-NOTES-1.22
index 0e2f1da..6720674 100644 (file)
--- a/RELEASE-NOTES-1.22
+++ b/RELEASE-NOTES-1.22
@@ -55,6 +55,7 @@ production.
 * Pager's properly validate which fields are allowed to be sorted on.
 * mw.util.tooltipAccessKeyRegexp: The regex now matches "option-" as well.
   Support for Mac "option" was added in 1.16, but the regex was never updated.
+* (bug 46768) Usernames of blocking users now display correctly, even if numeric.
 
 === API changes in 1.22 ===
 * (bug 46626) xmldoublequote parameter was removed. Because of a bug, the

diff --git a/includes/Block.php b/includes/Block.php
index 638b9a6..eab1754 100644 (file)
--- a/includes/Block.php
+++ b/includes/Block.php
@@ -1393,4 +1393,43 @@ class Block {
        public function setBlocker( $user ) {
                $this->blocker = $user;
        }
+
+       /**
+        * Get the key and parameters for the corresponding error message.
+        *
+        * @since 1.22
+        * @param IContextSource $context
+        * @return array
+        */
+       public function getPermissionsError( IContextSource $context ) {
+               $blocker = $this->getBlocker();
+               if ( $blocker instanceof User ) { // local user
+                       $blockerUserpage = $blocker->getUserPage();
+                       $link = "[[{$blockerUserpage->getPrefixedText()}|{$blockerUserpage->getText()}]]";
+               } else { // foreign user
+                       $link = $blocker;
+               }
+
+               $reason = $this->mReason;
+               if ( $reason == '' ) {
+                       $reason = $context->msg( 'blockednoreason' )->text();
+               }
+
+               /* $ip returns who *is* being blocked, $intended contains who was meant to be blocked.
+                * This could be a username, an IP range, or a single IP. */
+               $intended = $this->getTarget();
+
+               $lang = $context->getLanguage();
+               return array(
+                       $this->mAuto ? 'autoblockedtext' : 'blockedtext',
+                       $link,
+                       $reason,
+                       $context->getRequest()->getIP(),
+                       $this->getByName(),
+                       $this->getId(),
+                       $lang->formatExpiry( $this->mExpiry ),
+                       (string)$intended,
+                       $lang->timeanddate( wfTimestamp( TS_MW, $this->mTimestamp ), true ),
+               );
+       }
 }

diff --git a/includes/Exception.php b/includes/Exception.php
index b42ae99..c513ef7 100644 (file)
--- a/includes/Exception.php
+++ b/includes/Exception.php
@@ -467,39 +467,9 @@ class ThrottledError extends ErrorPageError {
  */
 class UserBlockedError extends ErrorPageError {
        public function __construct( Block $block ) {
-               global $wgLang, $wgRequest;
-
-               $blocker = $block->getBlocker();
-               if ( $blocker instanceof User ) { // local user
-                       $blockerUserpage = $block->getBlocker()->getUserPage();
-                       $link = "[[{$blockerUserpage->getPrefixedText()}|{$blockerUserpage->getText()}]]";
-               } else { // foreign user
-                       $link = $blocker;
-               }
-
-               $reason = $block->mReason;
-               if ( $reason == '' ) {
-                       $reason = wfMessage( 'blockednoreason' )->text();
-               }
-
-               /* $ip returns who *is* being blocked, $intended contains who was meant to be blocked.
-                * This could be a username, an IP range, or a single IP. */
-               $intended = $block->getTarget();
-
-               parent::__construct(
-                       'blockedtitle',
-                       $block->mAuto ? 'autoblockedtext' : 'blockedtext',
-                       array(
-                               $link,
-                               $reason,
-                               $wgRequest->getIP(),
-                               $block->getByName(),
-                               $block->getId(),
-                               $wgLang->formatExpiry( $block->mExpiry ),
-                               $intended,
-                               $wgLang->timeanddate( wfTimestamp( TS_MW, $block->mTimestamp ), true )
-                       )
-               );
+               // @todo FIXME: Implement a more proper way to get context here.
+               $params = $block->getPermissionsError( RequestContext::getMain() );
+               parent::__construct( 'blockedtitle', array_shift( $params ), $params );
        }
 }
 

diff --git a/includes/Title.php b/includes/Title.php
index fda790f..cf34171 100644 (file)
--- a/includes/Title.php
+++ b/includes/Title.php
@@ -2056,38 +2056,8 @@ class Title {
                        // Don't block the user from editing their own talk page unless they've been
                        // explicitly blocked from that too.
                } elseif ( $user->isBlocked() && $user->mBlock->prevents( $action ) !== false ) {
-                       $block = $user->getBlock();
-
-                       // This is from OutputPage::blockedPage
-                       // Copied at r23888 by werdna
-
-                       $id = $user->blockedBy();
-                       $reason = $user->blockedFor();
-                       if ( $reason == '' ) {
-                               $reason = wfMessage( 'blockednoreason' )->text();
-                       }
-                       $ip = $user->getRequest()->getIP();
-
-                       if ( is_numeric( $id ) ) {
-                               $name = User::whoIs( $id );
-                       } else {
-                               $name = $id;
-                       }
-
-                       $link = '[[' . $wgContLang->getNsText( NS_USER ) . ":{$name}|{$name}]]";
-                       $blockid = $block->getId();
-                       $blockExpiry = $block->getExpiry();
-                       $blockTimestamp = $wgLang->timeanddate( wfTimestamp( TS_MW, $block->mTimestamp ), true );
-                       if ( $blockExpiry == 'infinity' ) {
-                               $blockExpiry = wfMessage( 'infiniteblock' )->text();
-                       } else {
-                               $blockExpiry = $wgLang->timeanddate( wfTimestamp( TS_MW, $blockExpiry ), true );
-                       }
-
-                       $intended = strval( $block->getTarget() );
-
-                       $errors[] = array( ( $block->mAuto ? 'autoblockedtext' : 'blockedtext' ), $link, $reason, $ip, $name,
-                               $blockid, $blockExpiry, $intended, $blockTimestamp );
+                       // @todo FIXME: Pass the relevant context into this function.
+                       $errors[] = $user->getBlock()->getPermissionsError( RequestContext::getMain() );
                }
 
                return $errors;

diff --git a/languages/Language.php b/languages/Language.php
index 24d083d..9651f3d 100644 (file)
--- a/languages/Language.php
+++ b/languages/Language.php
@@ -4139,15 +4139,14 @@ class Language {
         * @since 1.18
         */
        public function formatExpiry( $expiry, $format = true ) {
-               static $infinity, $infinityMsg;
+               static $infinity;
                if ( $infinity === null ) {
-                       $infinityMsg = wfMessage( 'infiniteblock' );
                        $infinity = wfGetDB( DB_SLAVE )->getInfinity();
                }
 
                if ( $expiry == '' || $expiry == $infinity ) {
                        return $format === true
-                               ? $infinityMsg
+                               ? $this->getMessageFromDB( 'infiniteblock' )
                                : $infinity;
                } else {
                        return $format === true

diff --git a/tests/phpunit/MediaWikiLangTestCase.php b/tests/phpunit/MediaWikiLangTestCase.php
index 0cf6e38..1131385 100644 (file)
--- a/tests/phpunit/MediaWikiLangTestCase.php
+++ b/tests/phpunit/MediaWikiLangTestCase.php
@@ -15,6 +15,10 @@ abstract class MediaWikiLangTestCase extends MediaWikiTestCase {
                                "\$wgContLang->getCode() (" . $wgContLang->getCode() . ")" );
                }
 
+               // HACK: Call getLanguage() so the real $wgContLang is cached as the user language
+               // rather than our fake one. This is to avoid breaking other, unrelated tests.
+               RequestContext::getMain()->getLanguage();
+
                $langCode = 'en'; # For mainpage to be 'Main Page'
                $langObj = Language::factory( $langCode );
 

diff --git a/tests/phpunit/includes/TitlePermissionTest.php b/tests/phpunit/includes/TitlePermissionTest.php
index e2c079a..aaf81f6 100644 (file)
--- a/tests/phpunit/includes/TitlePermissionTest.php
+++ b/tests/phpunit/includes/TitlePermissionTest.php
@@ -648,7 +648,8 @@ class TitlePermissionTest extends MediaWikiLangTestCase {
                global $wgLocalTZoffset;
                $wgLocalTZoffset = -60;
                $this->user->mBlockedby = $this->user->getName();
-               $this->user->mBlock = new Block( '127.0.8.1', 0, 1, 'no reason given', $now, 0, 10 );
+               $this->user->mBlock = new Block( '127.0.8.1', 0, $this->user->getId(),
+                       'no reason given', $now, 0, 10 );
                $this->assertEquals( array( array( 'blockedtext',
                                '[[User:Useruser|Useruser]]', 'no reason given', '127.0.0.1',
                                'Useruser', null, '23:00, 31 December 1969', '127.0.8.1',