r102024 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r102023‎ | r102024 | r102025 >
Date:18:35, 4 November 2011
Author:ialex
Status:resolved (Comments)
Tags:
Comment:
Some cleanup to EditPage:
* Added displayPermissionsError() to show the permissions error page with the content if there's content to show or it will throw an normal exception if there's nothing to display; this restores a functionnality lost at some point and added new message 'viewyourtext' that is displayed when the user made changes to the content (e.g. when the block happens while the user is editing the page)
* Marked EditPage::readOnlyPage() as deprecated since displayPermissionsError() is now used. This also means that OutputPage::readOnlyPage() is no longer used to display such error pages (but still have to keep that functionnality since it's used by extensions, same for EditPage::readOnlyPage(), *sigh*)
* Removed blockedPage()'s display of content since it's nearly never called (a blocked user would trigger the path mentionned above, not the check in internalAttemptSave()) and also removed 'blockedoriginalsource' and 'blockededitsource' messages that are no longer used
* Use OutputPage::prepareErrorPage() to prepare error pages instead of doing all that stuff directly
* Moved noCreatePermission() near other error-related functions
* Throw an exception directly in attemptSave() instead of calling deprecated methods and marked noCreatePermission() and blockedPage() as deprecated
Modified paths:
  • /trunk/phase3/includes/EditPage.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesEn.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesQqq.php (modified) (history)
  • /trunk/phase3/maintenance/language/messages.inc (modified) (history)

Diff [purge]

Index: trunk/phase3/maintenance/language/messages.inc
@@ -398,6 +398,7 @@
399399 'actionthrottledtext',
400400 'protectedpagetext',
401401 'viewsourcetext',
 402+ 'viewyourtext',
402403 'protectedinterface',
403404 'editinginterface',
404405 'sqlhidden',
@@ -587,8 +588,6 @@
588589 'blockedtext',
589590 'autoblockedtext',
590591 'blockednoreason',
591 - 'blockedoriginalsource',
592 - 'blockededitsource',
593592 'whitelistedittitle',
594593 'whitelistedittext',
595594 'confirmedittext',
Index: trunk/phase3/includes/EditPage.php
@@ -403,27 +403,6 @@
404404 $this->preview = true;
405405 }
406406
407 - $wgOut->addModules( array( 'mediawiki.action.edit' ) );
408 -
409 - if ( $wgUser->getOption( 'uselivepreview', false ) ) {
410 - $wgOut->addModules( 'mediawiki.legacy.preview' );
411 - }
412 - // Bug #19334: textarea jumps when editing articles in IE8
413 - $wgOut->addStyle( 'common/IE80Fixes.css', 'screen', 'IE 8' );
414 -
415 - $permErrors = $this->getEditPermissionErrors();
416 - if ( $permErrors ) {
417 - // Auto-block user's IP if the account was "hard" blocked
418 - $wgUser->spreadAnyEditBlock();
419 -
420 - wfDebug( __METHOD__ . ": User can't edit\n" );
421 - $content = $this->getContent( null );
422 - $content = $content === '' ? null : $content;
423 - $this->readOnlyPage( $content, true, $permErrors, 'edit' );
424 - wfProfileOut( __METHOD__ );
425 - return;
426 - }
427 -
428407 if ( $this->save ) {
429408 $this->formtype = 'save';
430409 } elseif ( $this->preview ) {
@@ -439,6 +418,26 @@
440419 }
441420 }
442421
 422+ $permErrors = $this->getEditPermissionErrors();
 423+ if ( $permErrors ) {
 424+ wfDebug( __METHOD__ . ": User can't edit\n" );
 425+ // Auto-block user's IP if the account was "hard" blocked
 426+ $wgUser->spreadAnyEditBlock();
 427+
 428+ $this->displayPermissionsError( $permErrors );
 429+
 430+ wfProfileOut( __METHOD__ );
 431+ return;
 432+ }
 433+
 434+ $wgOut->addModules( array( 'mediawiki.action.edit' ) );
 435+
 436+ if ( $wgUser->getOption( 'uselivepreview', false ) ) {
 437+ $wgOut->addModules( 'mediawiki.legacy.preview' );
 438+ }
 439+ // Bug #19334: textarea jumps when editing articles in IE8
 440+ $wgOut->addStyle( 'common/IE80Fixes.css', 'screen', 'IE 8' );
 441+
443442 wfProfileIn( __METHOD__."-business-end" );
444443
445444 $this->isConflict = false;
@@ -540,9 +539,68 @@
541540 }
542541
543542 /**
 543+ * Display a permissions error page, like OutputPage::showPermissionsErrorPage(),
 544+ * but with the following differences:
 545+ * - If redlink=1, the user will be redirect to the page
 546+ * - If there is content to display or the error occurs while either saving,
 547+ * previewing or showing the difference, it will be a
 548+ * "View source for ..." page displaying the source code after the error message.
 549+ *
 550+ * @since 1.19
 551+ * @param $permErrors Array of permissions errors, as returned by
 552+ * Title::getUserPermissionsErrors().
 553+ */
 554+ protected function displayPermissionsError( array $permErrors ) {
 555+ global $wgRequest, $wgOut;
 556+
 557+ if ( $wgRequest->getBool( 'redlink' ) ) {
 558+ // The edit page was reached via a red link.
 559+ // Redirect to the article page and let them click the edit tab if
 560+ // they really want a permission error.
 561+ $wgOut->redirect( $this->mTitle->getFullUrl() );
 562+ return;
 563+ }
 564+
 565+ $content = $this->getContent();
 566+
 567+ # Use the normal message if there's nothing to display
 568+ if ( $this->firsttime && $content === '' ) {
 569+ $action = $this->mTitle->exists() ? 'edit' :
 570+ ( $permission = $this->mTitle->isTalkPage() ? 'createtalk' : 'createpage' );
 571+ throw new PermissionsError( $action, $permErrors );
 572+ }
 573+
 574+ $wgOut->setPageTitle( wfMessage( 'viewsource' ) );
 575+ $wgOut->setSubtitle(
 576+ wfMessage( 'viewsourcefor', Linker::linkKnown( $this->mTitle ) )->text()
 577+ );
 578+ $wgOut->addWikiText( $wgOut->formatPermissionsErrorMessage( $permErrors, 'edit' ) );
 579+ $wgOut->addHTML( "<hr />\n" );
 580+
 581+ # If the user made changes, preserve them when showing the markup
 582+ # (This happens when a user is blocked during edit, for instance)
 583+ if ( !$this->firsttime ) {
 584+ $content = $this->textbox1;
 585+ $wgOut->addWikiMsg( 'viewyourtext' );
 586+ } else {
 587+ $wgOut->addWikiMsg( 'viewsourcetext' );
 588+ }
 589+
 590+ $this->showTextbox( $content, 'wpTextbox1', array( 'readonly' ) );
 591+
 592+ $wgOut->addHTML( Html::rawElement( 'div', array( 'class' => 'templatesUsed' ),
 593+ Linker::formatTemplates( $this->getTemplates() ) ) );
 594+
 595+ if ( $this->mTitle->exists() ) {
 596+ $wgOut->returnToMain( null, $this->mTitle );
 597+ }
 598+ }
 599+
 600+ /**
544601 * Show a read-only error
545602 * Parameters are the same as OutputPage:readOnlyPage()
546603 * Redirect to the article page if redlink=1
 604+ * @deprecated in 1.19; use displayPermissionsError() instead
547605 */
548606 function readOnlyPage( $source = null, $protected = false, $reasons = array(), $action = null ) {
549607 global $wgRequest, $wgOut;
@@ -2188,26 +2246,13 @@
21892247
21902248 /**
21912249 * Call the stock "user is blocked" page
 2250+ *
 2251+ * @deprecated in 1.19; throw an exception directly instead
21922252 */
21932253 function blockedPage() {
2194 - global $wgOut;
2195 - $wgOut->blockedPage( false ); # Standard block notice on the top, don't 'return'
 2254+ global $wgUser;
21962255
2197 - # If the user made changes, preserve them when showing the markup
2198 - # (This happens when a user is blocked during edit, for instance)
2199 - $first = $this->firsttime || ( !$this->save && $this->textbox1 == '' );
2200 - if ( $first ) {
2201 - $source = $this->mTitle->exists() ? $this->getContent() : false;
2202 - } else {
2203 - $source = $this->textbox1;
2204 - }
2205 -
2206 - # Spit out the source or the user's modified version
2207 - if ( $source !== false ) {
2208 - $wgOut->addHTML( '<hr />' );
2209 - $wgOut->addWikiMsg( $first ? 'blockedoriginalsource' : 'blockededitsource', $this->mTitle->getPrefixedText() );
2210 - $this->showTextbox1( array( 'readonly' ), $source );
2211 - }
 2256+ throw new UserBlockedError( $wgUser->mBlock );
22122257 }
22132258
22142259 /**
@@ -2216,6 +2261,8 @@
22172262 function userNotLoggedInPage() {
22182263 global $wgOut;
22192264
 2265+ $wgOut->prepareErrorPage( wfMessage( 'whitelistedittitle' ) );
 2266+
22202267 $loginTitle = SpecialPage::getTitleFor( 'Userlogin' );
22212268 $loginLink = Linker::linkKnown(
22222269 $loginTitle,
@@ -2223,25 +2270,28 @@
22242271 array(),
22252272 array( 'returnto' => $this->getContextTitle()->getPrefixedText() )
22262273 );
2227 -
2228 - $wgOut->setPageTitle( wfMessage( 'whitelistedittitle' ) );
2229 - $wgOut->setRobotPolicy( 'noindex,nofollow' );
2230 - $wgOut->setArticleRelated( false );
2231 -
22322274 $wgOut->addHTML( wfMessage( 'whitelistedittext' )->rawParams( $loginLink )->parse() );
22332275 $wgOut->returnToMain( false, $this->getContextTitle() );
22342276 }
22352277
22362278 /**
 2279+ * Show an error page saying to the user that he has insufficient permissions
 2280+ * to create a new page
 2281+ *
 2282+ * @deprecated in 1.19; throw an exception directly instead
 2283+ */
 2284+ function noCreatePermission() {
 2285+ throw new MWException( 'nocreatetitle', 'nocreatetext' );
 2286+ }
 2287+
 2288+ /**
22372289 * Creates a basic error page which informs the user that
22382290 * they have attempted to edit a nonexistent section.
22392291 */
22402292 function noSuchSectionPage() {
22412293 global $wgOut;
22422294
2243 - $wgOut->setPageTitle( wfMessage( 'nosuchsectiontitle' ) );
2244 - $wgOut->setRobotPolicy( 'noindex,nofollow' );
2245 - $wgOut->setArticleRelated( false );
 2295+ $wgOut->prepareErrorPage( wfMessage( 'nosuchsectiontitle' ) );
22462296
22472297 $res = wfMsgExt( 'nosuchsectiontext', 'parse', $this->section );
22482298 wfRunHooks( 'EditPageNoSuchSection', array( &$this, &$res ) );
@@ -2259,9 +2309,7 @@
22602310 static function spamPage( $match = false ) {
22612311 global $wgOut, $wgTitle;
22622312
2263 - $wgOut->setPageTitle( wfMessage( 'spamprotectiontitle' ) );
2264 - $wgOut->setRobotPolicy( 'noindex,nofollow' );
2265 - $wgOut->setArticleRelated( false );
 2313+ $wgOut->prepareErrorPage( wfMessage( 'spamprotectiontitle' ) );
22662314
22672315 $wgOut->addHTML( '<div id="spamprotected">' );
22682316 $wgOut->addWikiMsg( 'spamprotectiontext' );
@@ -2282,9 +2330,7 @@
22832331 global $wgOut;
22842332 $this->textbox2 = $this->textbox1;
22852333
2286 - $wgOut->setPageTitle( wfMessage( 'spamprotectiontitle' ) );
2287 - $wgOut->setRobotPolicy( 'noindex,nofollow' );
2288 - $wgOut->setArticleRelated( false );
 2334+ $wgOut->prepareErrorPage( wfMessage( 'spamprotectiontitle' ) );
22892335
22902336 $wgOut->addHTML( '<div id="spamprotected">' );
22912337 $wgOut->addWikiMsg( 'spamprotectiontext' );
@@ -2841,12 +2887,6 @@
28422888 return strtr( $result, array( "&#x0" => "&#x" ) );
28432889 }
28442890
2845 - function noCreatePermission() {
2846 - global $wgOut;
2847 - $wgOut->setPageTitle( wfMessage( 'nocreatetitle' ) );
2848 - $wgOut->addWikiMsg( 'nocreatetext' );
2849 - }
2850 -
28512891 /**
28522892 * Attempt submission
28532893 * @return bool false if output is done, true if the rest of the form should be displayed
@@ -2901,42 +2941,39 @@
29022942 $wgOut->redirect( $this->mTitle->getFullURL( $extraQuery ) . $sectionanchor );
29032943 return false;
29042944
 2945+ case self::AS_BLANK_ARTICLE:
 2946+ $wgOut->redirect( $this->getContextTitle()->getFullURL() );
 2947+ return false;
 2948+
29052949 case self::AS_SPAM_ERROR:
29062950 $this->spamPageWithContent( $resultDetails['spam'] );
29072951 return false;
29082952
29092953 case self::AS_BLOCKED_PAGE_FOR_USER:
2910 - $this->blockedPage();
2911 - return false;
 2954+ throw new UserBlockedError( $wgUser->mBlock );
29122955
29132956 case self::AS_IMAGE_REDIRECT_ANON:
2914 - $wgOut->showErrorPage( 'uploadnologin', 'uploadnologintext' );
2915 - return false;
 2957+ throw new ErrorPageError( 'uploadnologin', 'uploadnologintext' );
29162958
 2959+ case self::AS_IMAGE_REDIRECT_LOGGED:
 2960+ throw new PermissionsError( 'upload' );
 2961+
29172962 case self::AS_READ_ONLY_PAGE_ANON:
29182963 $this->userNotLoggedInPage();
29192964 return false;
29202965
29212966 case self::AS_READ_ONLY_PAGE_LOGGED:
 2967+ throw new PermissionsError( 'edit' );
 2968+
29222969 case self::AS_READ_ONLY_PAGE:
2923 - $wgOut->readOnlyPage();
2924 - return false;
 2970+ throw new ReadOnlyError;
29252971
29262972 case self::AS_RATE_LIMITED:
2927 - $wgOut->rateLimited();
2928 - return false;
 2973+ throw new ThrottledError();
29292974
29302975 case self::AS_NO_CREATE_PERMISSION:
2931 - $this->noCreatePermission();
2932 - return false;
 2976+ throw new MWException( 'nocreatetitle', 'nocreatetext' );
29332977
2934 - case self::AS_BLANK_ARTICLE:
2935 - $wgOut->redirect( $this->getContextTitle()->getFullURL() );
2936 - return false;
2937 -
2938 - case self::AS_IMAGE_REDIRECT_LOGGED:
2939 - $wgOut->permissionRequired( 'upload' );
2940 - return false;
29412978 }
29422979 return false;
29432980 }
Index: trunk/phase3/languages/messages/MessagesQqq.php
@@ -660,6 +660,7 @@
661661
662662 * $1: the protection type, e.g. "protect" for fully protected pages',
663663 'viewsourcetext' => 'The text shown when displaying the source of a page that the user has no permission to edit',
 664+'viewyourtext' => 'Same as {{msg-mw|viewsourcetext}} but when showing the text submitted by the user, this happens e.g. when the user was blocked while he is editing the page',
664665 'protectedinterface' => 'Message shown if a user without the "editinterface" right tries to edit a page in the MediaWiki namespace.',
665666 'editinginterface' => "A message shown when editing pages in the namespace MediaWiki:. In the [http://translatewiki.net/wiki/Main_Page?setlang=en URL], '''change \"setlang=en\" to your own language code.'''",
666667 'ns-specialprotected' => 'Error message displayed when trying to edit a page in the Special namespace',
Index: trunk/phase3/languages/messages/MessagesEn.php
@@ -1011,6 +1011,7 @@
10121012 Please try again in a few minutes.',
10131013 'protectedpagetext' => 'This page has been protected to prevent editing.',
10141014 'viewsourcetext' => 'You can view and copy the source of this page:',
 1015+'viewyourtext' => "You can view and copy the source of '''your edits''' to this page:",
10151016 'protectedinterface' => 'This page provides interface text for the software, and is protected to prevent abuse.',
10161017 'editinginterface' => "'''Warning:''' You are editing a page which is used to provide interface text for the software.
10171018 Changes to this page will affect the appearance of the user interface for other users.
@@ -1297,8 +1298,6 @@
12981299 Your current IP address is $3, and the block ID is #$5.
12991300 Please include all above details in any queries you make.',
13001301 'blockednoreason' => 'no reason given',
1301 -'blockedoriginalsource' => "The source of '''$1''' is shown below:",
1302 -'blockededitsource' => "The text of '''your edits''' to '''$1''' is shown below:",
13031302 'whitelistedittitle' => 'Login required to edit',
13041303 'whitelistedittext' => 'You have to $1 to edit pages.',
13051304 'confirmedittext' => 'You must confirm your e-mail address before editing pages.

Sign-offs

UserFlagDate
Werdnainspected07:04, 6 November 2011

Follow-up revisions

RevisionCommit summaryAuthorDate
r107204Per Nikerabbit, follow-up r102024: removed uneeded variableialex15:32, 24 December 2011

Comments

#Comment by Werdna (talk | contribs)   07:04, 6 November 2011

Shiny!

#Comment by Nikerabbit (talk | contribs)   12:53, 24 December 2011
+			$action = $this->mTitle->exists() ? 'edit' :
+				( $permission = $this->mTitle->isTalkPage() ? 'createtalk' : 'createpage' );

Permission variable is unused as far as I can see.

+				throw new MWException( 'nocreatetitle', 'nocreatetext' );

Should that be ErrorPageError?

#Comment by Nikerabbit (talk | contribs)   12:55, 24 December 2011

I see that the latter has been changed in r102112.

#Comment by IAlex (talk | contribs)   15:33, 24 December 2011

Done in r107204.

Status & tagging log