r86407 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r86406‎ | r86407 | r86408 >
Date:15:45, 19 April 2011
Author:happy-melon
Status:resolved (Comments)
Tags:
Comment:
Follow-up r86255: don't special-case redirecting special pages in executePath(), make them subclass a RedirectSpecialPage and have their execute() method do the redirection. Fixes fatal errors seen on TWN where executePath() was trying to call SpecialMyPage::execute(), which bubbled up to SpecialPage::execute() and made a horrible mess.
Modified paths:
  • /trunk/phase3/includes/AutoLoader.php (modified) (history)
  • /trunk/phase3/includes/SpecialPage.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/AutoLoader.php
@@ -191,6 +191,7 @@
192192 'RdfMetaData' => 'includes/Metadata.php',
193193 'ReadOnlyError' => 'includes/Exception.php',
194194 'RecentChange' => 'includes/RecentChange.php',
 195+ 'RedirectSpecialPage' => 'includes/SpecialPage.php',
195196 'RegexlikeReplacer' => 'includes/StringUtils.php',
196197 'ReplacementArray' => 'includes/StringUtils.php',
197198 'Replacer' => 'includes/StringUtils.php',
@@ -206,6 +207,9 @@
207208 'Skin' => 'includes/Skin.php',
208209 'SkinLegacy' => 'includes/SkinLegacy.php',
209210 'SkinTemplate' => 'includes/SkinTemplate.php',
 211+ 'SpecialCreateAccount' => 'includes/SpecialPage.php',
 212+ 'SpecialListAdmins' => 'includes/SpecialPage.php',
 213+ 'SpecialListBots' => 'includes/SpecialPage.php',
210214 'SpecialMycontributions' => 'includes/SpecialPage.php',
211215 'SpecialMypage' => 'includes/SpecialPage.php',
212216 'SpecialMytalk' => 'includes/SpecialPage.php',
Index: trunk/phase3/includes/SpecialPage.php
@@ -485,7 +485,7 @@
486486 * @param $user User: the user to check
487487 * @return Boolean: does the user have permission to view the page?
488488 */
489 - public function userCanExecute( $user ) {
 489+ public function userCanExecute( User $user ) {
490490 return $user->isAllowed( $this->mRestriction );
491491 }
492492
@@ -693,22 +693,32 @@
694694 * Shortcut to construct a special page alias.
695695 * @ingroup SpecialPage
696696 */
697 -abstract class SpecialRedirectToSpecial extends UnlistedSpecialPage {
 697+abstract class RedirectSpecialPage extends UnlistedSpecialPage {
698698
699699 // Query parameters that can be passed through redirects
700 - private $mAllowedRedirectParams = array();
 700+ protected $mAllowedRedirectParams = array();
701701
702702 // Query parameteres added by redirects
703 - private $mAddedRedirectParams = array();
704 -
705 - var $redirName, $redirSubpage;
706 -
707 - function __construct( $name, $redirName, $redirSubpage = false, $allowedRedirectParams = array(), $addedRedirectParams = array() ) {
708 - parent::__construct( $name );
709 - $this->redirName = $redirName;
710 - $this->redirSubpage = $redirSubpage;
711 - $this->mAllowedRedirectParams = $allowedRedirectParams;
712 - $this->mAddedRedirectParams = $addedRedirectParams;
 703+ protected $mAddedRedirectParams = array();
 704+
 705+ public function execute( $par ){
 706+ $redirect = $this->getRedirect( $par );
 707+ $query = $this->getRedirectQuery();
 708+ if ( $redirect instanceof Title ) {
 709+ $url = $redirect->getFullUrl( $query );
 710+ $this->getContext()->output->redirect( $url );
 711+ wfProfileOut( __METHOD__ );
 712+ return $redirect;
 713+ } elseif ( $redirect === true ) {
 714+ global $wgScript;
 715+ $url = $wgScript . '?' . wfArrayToCGI( $query );
 716+ $this->getContext()->output->redirect( $url );
 717+ wfProfileOut( __METHOD__ );
 718+ return $redirect;
 719+ } else {
 720+ $class = __CLASS__;
 721+ throw new MWException( "RedirectSpecialPage $class doesn't redirect!" );
 722+ }
713723 }
714724
715725 /**
@@ -717,13 +727,7 @@
718728 *
719729 * @return Title|false
720730 */
721 - public function getRedirect( $subpage ) {
722 - if ( $this->redirSubpage === false ) {
723 - return SpecialPage::getTitleFor( $this->redirName, $subpage );
724 - } else {
725 - return SpecialPage::getTitleFor( $this->redirName, $this->redirSubpage );
726 - }
727 - }
 731+ abstract public function getRedirect( $par );
728732
729733 /**
730734 * Return part of the request string for a special redirect page
@@ -750,6 +754,27 @@
751755 }
752756 }
753757
 758+abstract class SpecialRedirectToSpecial extends RedirectSpecialPage {
 759+
 760+ var $redirName, $redirSubpage;
 761+
 762+ function __construct( $name, $redirName, $redirSubpage = false, $allowedRedirectParams = array(), $addedRedirectParams = array() ) {
 763+ parent::__construct( $name );
 764+ $this->redirName = $redirName;
 765+ $this->redirSubpage = $redirSubpage;
 766+ $this->mAllowedRedirectParams = $allowedRedirectParams;
 767+ $this->mAddedRedirectParams = $addedRedirectParams;
 768+ }
 769+
 770+ public function getRedirect( $subpage ) {
 771+ if ( $this->redirSubpage === false ) {
 772+ return SpecialPage::getTitleFor( $this->redirName, $subpage );
 773+ } else {
 774+ return SpecialPage::getTitleFor( $this->redirName, $this->redirSubpage );
 775+ }
 776+ }
 777+}
 778+
754779 /**
755780 * ListAdmins --> ListUsers/admin
756781 */
@@ -789,7 +814,7 @@
790815 * Shortcut to construct a special page pointing to current user user's page.
791816 * @ingroup SpecialPage
792817 */
793 -class SpecialMypage extends UnlistedSpecialPage {
 818+class SpecialMypage extends RedirectSpecialPage {
794819 function __construct() {
795820 parent::__construct( 'Mypage' );
796821 $this->mAllowedRedirectParams = array( 'action' , 'preload' , 'editintro',
@@ -810,7 +835,7 @@
811836 * Shortcut to construct a special page pointing to current user talk page.
812837 * @ingroup SpecialPage
813838 */
814 -class SpecialMytalk extends UnlistedSpecialPage {
 839+class SpecialMytalk extends RedirectSpecialPage {
815840 function __construct() {
816841 parent::__construct( 'Mytalk' );
817842 $this->mAllowedRedirectParams = array( 'action' , 'preload' , 'editintro',
@@ -831,7 +856,7 @@
832857 * Shortcut to construct a special page pointing to current user contributions.
833858 * @ingroup SpecialPage
834859 */
835 -class SpecialMycontributions extends UnlistedSpecialPage {
 860+class SpecialMycontributions extends RedirectSpecialPage {
836861 function __construct() {
837862 parent::__construct( 'Mycontributions' );
838863 $this->mAllowedRedirectParams = array( 'limit', 'namespace', 'tagfilter',
@@ -847,7 +872,7 @@
848873 /**
849874 * Redirect to Special:Listfiles?user=$wgUser
850875 */
851 -class SpecialMyuploads extends UnlistedSpecialPage {
 876+class SpecialMyuploads extends RedirectSpecialPage {
852877 function __construct() {
853878 parent::__construct( 'Myuploads' );
854879 $this->mAllowedRedirectParams = array( 'limit' );
@@ -862,7 +887,7 @@
863888 /**
864889 * Redirect from Special:PermanentLink/### to index.php?oldid=###
865890 */
866 -class SpecialPermanentLink extends UnlistedSpecialPage {
 891+class SpecialPermanentLink extends RedirectSpecialPage {
867892 function __construct() {
868893 parent::__construct( 'PermanentLink' );
869894 $this->mAllowedRedirectParams = array();

Follow-up revisions

RevisionCommit summaryAuthorDate
r86481Follow-up r86407: missed a file (and some random documentation); I'm surprise...happy-melon15:26, 20 April 2011
r86505Follow-up r86407: Add type hinting to SpecialPage::userCanExecute() overrides...happy-melon17:07, 20 April 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r86255Refactor the factory/i18n/list/etc static methods from SpecialPage into their...happy-melon11:31, 17 April 2011

Comments

#Comment by Happy-melon (talk | contribs)   00:10, 20 April 2011

Adding the type-hinting to SpecialPage::userCanExecute() is throwing E_STRICTs everywhere.

Status & tagging log