r102112 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r102111‎ | r102112 | r102113 >
Date:19:51, 5 November 2011
Author:ialex
Status:ok (Comments)
Tags:
Comment:
* Made OuputPage::showPermissionsErrorPage() show a different messages for 'read', 'edit', 'create' and 'upload' actions to saying "You need to log in to do this action" when 1) The user is not logged in 2) The only error is a permissions error (no block or something else) and 3) The error can simply be avoided by logging in
* This replaces OuputPage::loginToUse() functionnality, made it simply throw a PermissionsEror exception and updated all calls in core
* Same for the check in SpecialUpload::execute(), EditPage::userNotLoggedInPage() and EditPage::noCreatePermission()
* Throw the same exception in EditPage::attemptSave() whether the user is logged in or not and let OuputPage::showPermissionsErrorPage() decide which message to display
* Replaced call to deprecated OutputPage::blockedPage() in SpecialUpload
* Displayed messages are the same as now, except the title is always "loginreqtitle"
* 'nocreatetitle' and 'uploadnologin' messages are still used by extensions, so I kept them, but the message 'whitelistedittitle' is not used anymore and has been removed
Modified paths:
  • /trunk/phase3/includes/Article.php (modified) (history)
  • /trunk/phase3/includes/EditPage.php (modified) (history)
  • /trunk/phase3/includes/OutputPage.php (modified) (history)
  • /trunk/phase3/includes/Wiki.php (modified) (history)
  • /trunk/phase3/includes/diff/DifferenceEngine.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialUpload.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesEn.php (modified) (history)
  • /trunk/phase3/maintenance/language/messages.inc (modified) (history)

Diff [purge]

Index: trunk/phase3/maintenance/language/messages.inc
——@@ -588,7 +588,6 @@
589589 'blockedtext',
590590 'autoblockedtext',
591591 'blockednoreason',
592 - 'whitelistedittitle',
593592 'whitelistedittext',
594593 'confirmedittext',
595594 'nosuchsectiontitle',
Index: trunk/phase3/includes/diff/DifferenceEngine.php
——@@ -196,11 +196,8 @@
197197
198198 # mOldPage might not be set, see below.
199199 if ( !$this->mNewPage->userCanRead() || ( $this->mOldPage && !$this->mOldPage->userCanRead() ) ) {
200 - $wgOut->loginToUse();
201 - $wgOut->output();
202 - $wgOut->disable();
203200 wfProfileOut( __METHOD__ );
204 - return;
 201+ throw new PermissionsError( 'read' );
205202 }
206203
207204 # If external diffs are enabled both globally and for the user,
Index: trunk/phase3/includes/Article.php
——@@ -479,11 +479,8 @@
480480 # Another whitelist check in case oldid is altering the title
481481 if ( !$this->getTitle()->userCanRead() ) {
482482 wfDebug( __METHOD__ . ": denied on secondary read check\n" );
483 - $wgOut->loginToUse();
484 - $wgOut->output();
485 - $wgOut->disable();
486483 wfProfileOut( __METHOD__ );
487 - return;
 484+ throw new PermissionsError( 'read' );
488485 }
489486
490487 # Are we looking at an old revision
Index: trunk/phase3/includes/EditPage.php
——@@ -2257,21 +2257,11 @@
22582258
22592259 /**
22602260 * Produce the stock "please login to edit pages" page
 2261+ *
 2262+ * @deprecated in 1.19; throw an exception directly instead
22612263 */
22622264 function userNotLoggedInPage() {
2263 - global $wgOut;
2264 -
2265 - $wgOut->prepareErrorPage( wfMessage( 'whitelistedittitle' ) );
2266 -
2267 - $loginTitle = SpecialPage::getTitleFor( 'Userlogin' );
2268 - $loginLink = Linker::linkKnown(
2269 - $loginTitle,
2270 - wfMsgHtml( 'loginreqlink' ),
2271 - array(),
2272 - array( 'returnto' => $this->getContextTitle()->getPrefixedText() )
2273 - );
2274 - $wgOut->addHTML( wfMessage( 'whitelistedittext' )->rawParams( $loginLink )->parse() );
2275 - $wgOut->returnToMain( false, $this->getContextTitle() );
 2265+ throw new PermissionsError( 'edit' );
22762266 }
22772267
22782268 /**
——@@ -2281,7 +2271,8 @@
22822272 * @deprecated in 1.19; throw an exception directly instead
22832273 */
22842274 function noCreatePermission() {
2285 - throw new MWException( 'nocreatetitle', 'nocreatetext' );
 2275+ $permission = $this->mTitle->isTalkPage() ? 'createtalk' : 'createpage';
 2276+ throw new PermissionsError( $permission );
22862277 }
22872278
22882279 /**
——@@ -2953,15 +2944,10 @@
29542945 throw new UserBlockedError( $wgUser->mBlock );
29552946
29562947 case self::AS_IMAGE_REDIRECT_ANON:
2957 - throw new ErrorPageError( 'uploadnologin', 'uploadnologintext' );
2958 -
29592948 case self::AS_IMAGE_REDIRECT_LOGGED:
29602949 throw new PermissionsError( 'upload' );
29612950
29622951 case self::AS_READ_ONLY_PAGE_ANON:
2963 - $this->userNotLoggedInPage();
2964 - return false;
2965 -
29662952 case self::AS_READ_ONLY_PAGE_LOGGED:
29672953 throw new PermissionsError( 'edit' );
29682954
——@@ -2972,7 +2958,8 @@
29732959 throw new ThrottledError();
29742960
29752961 case self::AS_NO_CREATE_PERMISSION:
2976 - throw new MWException( 'nocreatetitle', 'nocreatetext' );
 2962+ $permission = $this->mTitle->isTalkPage() ? 'createtalk' : 'createpage';
 2963+ throw new PermissionsError( $permission );
29772964
29782965 }
29792966 return false;
Index: trunk/phase3/includes/OutputPage.php
——@@ -1979,9 +1979,64 @@
19801980 * @param $action String: action that was denied or null if unknown
19811981 */
19821982 public function showPermissionsErrorPage( $errors, $action = null ) {
1983 - $this->prepareErrorPage( $this->msg( 'permissionserrors' ) );
 1983+ global $wgGroupPermissions;
19841984
1985 - $this->addWikiText( $this->formatPermissionsErrorMessage( $errors, $action ) );
 1985+ // For some action (read, edit, create and upload), display a "login to do this action"
 1986+ // error if all of the following conditions are met:
 1987+ // 1. the user is not logged in
 1988+ // 2. the only error is insufficient permissions (i.e. no block or something else)
 1989+ // 3. the error can be avoided simply by logging in
 1990+ if ( in_array( $action, array( 'read', 'edit', 'createpage', 'createtalk', 'upload' ) )
 1991+ && $this->getUser()->isAnon() && count( $errors ) == 1 && isset( $errors[0][0] )
 1992+ && ( $errors[0][0] == 'badaccess-groups' || $errors[0][0] == 'badaccess-group0' )
 1993+ && ( ( isset( $wgGroupPermissions['user'][$action] ) && $wgGroupPermissions['user'][$action] )
 1994+ || ( isset( $wgGroupPermissions['autoconfirmed'][$action] ) && $wgGroupPermissions['autoconfirmed'][$action] ) )
 1995+ ) {
 1996+ $displayReturnto = null;
 1997+ $returnto = $this->getTitle();
 1998+ if ( $action == 'edit' ) {
 1999+ $msg = 'whitelistedittext';
 2000+ $displayReturnto = $returnto;
 2001+ } elseif ( $action == 'createpage' || $action == 'createtalk' ) {
 2002+ $msg = 'nocreatetext';
 2003+ } elseif ( $action == 'upload' ) {
 2004+ $msg = 'uploadnologintext';
 2005+ } else { # Read
 2006+ $msg = 'loginreqpagetext';
 2007+ $displayReturnto = Title::newMainPage();
 2008+ }
 2009+
 2010+ $query = array();
 2011+ if ( $returnto ) {
 2012+ $query['returnto'] = $returnto->getPrefixedText();
 2013+ $request = $this->getRequest();
 2014+ if ( !$request->wasPosted() ) {
 2015+ $returntoquery = $request->getValues();
 2016+ unset( $returntoquery['title'] );
 2017+ unset( $returntoquery['returnto'] );
 2018+ unset( $returntoquery['returntoquery'] );
 2019+ $query['returntoquery'] = wfArrayToCGI( $returntoquery );
 2020+ }
 2021+ }
 2022+ $loginLink = Linker::linkKnown(
 2023+ SpecialPage::getTitleFor( 'Userlogin' ),
 2024+ $this->msg( 'loginreqlink' )->escaped(),
 2025+ array(),
 2026+ $query
 2027+ );
 2028+
 2029+ $this->prepareErrorPage( $this->msg( 'loginreqtitle' ) );
 2030+ $this->addHTML( $this->msg( $msg )->rawParams( $loginLink )->parse() );
 2031+
 2032+ # Don't return to a page the user can't read otherwise
 2033+ # we'll end up in a pointless loop
 2034+ if ( $displayReturnto && $displayReturnto->userCanRead() ) {
 2035+ $this->returnToMain( null, $displayReturnto );
 2036+ }
 2037+ } else {
 2038+ $this->prepareErrorPage( $this->msg( 'permissionserrors' ) );
 2039+ $this->addWikiText( $this->formatPermissionsErrorMessage( $errors, $action ) );
 2040+ }
19862041 }
19872042
19882043 /**
——@@ -2008,29 +2063,11 @@
20092064
20102065 /**
20112066 * Produce the stock "please login to use the wiki" page
 2067+ *
 2068+ * @deprecated in 1.19; throw the exception directly
20122069 */
20132070 public function loginToUse() {
2014 - if( $this->getUser()->isLoggedIn() ) {
2015 - throw new PermissionsError( 'read' );
2016 - }
2017 -
2018 - $this->prepareErrorPage( $this->msg( 'loginreqtitle' ), $this->msg( 'errorpagetitle' ) );
2019 -
2020 - $loginLink = Linker::linkKnown(
2021 - SpecialPage::getTitleFor( 'Userlogin' ),
2022 - $this->msg( 'loginreqlink' )->escaped(),
2023 - array(),
2024 - array( 'returnto' => $this->getTitle()->getPrefixedText() )
2025 - );
2026 - $this->addHTML( $this->msg( 'loginreqpagetext' )->rawParams( $loginLink )->parse() .
2027 - "\n<!--" . $this->getTitle()->getPrefixedUrl() . '-->' );
2028 -
2029 - # Don't return to the main page if the user can't read it
2030 - # otherwise we'll end up in a pointless loop
2031 - $mainPage = Title::newMainPage();
2032 - if( $mainPage->userCanRead() ) {
2033 - $this->returnToMain( null, $mainPage );
2034 - }
 2071+ throw new PermissionsError( 'read' );
20352072 }
20362073
20372074 /**
Index: trunk/phase3/includes/Wiki.php
——@@ -161,7 +161,7 @@
162162 // the Read array in order for the user to see it. (We have to check here to
163163 // catch special pages etc. We check again in Article::view())
164164 } elseif ( !$title->userCanRead() ) {
165 - $output->loginToUse();
 165+ throw new PermissionsError( 'read' );
166166 // Interwiki redirects
167167 } elseif ( $title->getInterwiki() != '' ) {
168168 $rdfrom = $request->getVal( 'rdfrom' );
Index: trunk/phase3/includes/specials/SpecialUpload.php
——@@ -140,8 +140,6 @@
141141 * Special page entry point
142142 */
143143 public function execute( $par ) {
144 - global $wgGroupPermissions;
145 -
146144 $this->setHeaders();
147145 $this->outputHeader();
148146
——@@ -154,19 +152,12 @@
155153 $user = $this->getUser();
156154 $permissionRequired = UploadBase::isAllowed( $user );
157155 if( $permissionRequired !== true ) {
158 - if( !$user->isLoggedIn() && ( $wgGroupPermissions['user']['upload']
159 - || $wgGroupPermissions['autoconfirmed']['upload'] ) ) {
160 - // Custom message if logged-in users without any special rights can upload
161 - throw new ErrorPageError( 'uploadnologin', 'uploadnologintext' );
162 - } else {
163 - throw new PermissionsError( $permissionRequired );
164 - }
 156+ throw new PermissionsError( $permissionRequired );
165157 }
166158
167159 # Check blocks
168160 if( $user->isBlocked() ) {
169 - $this->getOutput()->blockedPage();
170 - return;
 161+ throw new UserBlockedError( $user->mBlock );
171162 }
172163
173164 # Check whether we actually want to allow changing stuff
Index: trunk/phase3/languages/messages/MessagesEn.php
——@@ -1298,7 +1298,6 @@
12991299 Your current IP address is $3, and the block ID is #$5.
13001300 Please include all above details in any queries you make.',
13011301 'blockednoreason' => 'no reason given',
1302 -'whitelistedittitle' => 'Login required to edit',
13031302 'whitelistedittext' => 'You have to $1 to edit pages.',
13041303 'confirmedittext' => 'You must confirm your e-mail address before editing pages.
13051304 Please set and validate your e-mail address through your [[Special:Preferences|user preferences]].',

Sign-offs

UserFlagDate
Werdnainspected06:47, 6 November 2011

Comments

#Comment by Werdna (talk | contribs)   06:47, 6 November 2011

I like this. :)

#Comment by Nikerabbit (talk | contribs)   09:23, 6 November 2011

What can we do about those messages only used by extensions, so that 1) they are not forgotten forever 2) are not removed while extensions still use them?

Status & tagging log