r92671 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r92670‎ | r92671 | r92672 >
Date:19:49, 20 July 2011
Author:ialex
Status:ok
Tags:
Comment:
* Use local context instead of global variables
* Call Linker methods statically
* Throw exceptions directly instead of calling OutputPage's methods that will throw them
* Use Title::getUserPermissionsErrors() instead of Title::userCan() so that the User object can be passed to them
Modified paths:
  • /trunk/phase3/includes/specials/SpecialMovepage.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/specials/SpecialMovepage.php
——@@ -42,55 +42,53 @@
4343 }
4444
4545 public function execute( $par ) {
46 - global $wgUser, $wgOut, $wgRequest;
47 -
4846 # Check for database lock
4947 if ( wfReadOnly() ) {
50 - $wgOut->readOnlyPage();
51 - return;
 48+ throw new ReadOnlyError;
5249 }
5350
5451 $this->setHeaders();
5552 $this->outputHeader();
5653
57 - $target = !is_null( $par ) ? $par : $wgRequest->getVal( 'target' );
 54+ $request = $this->getRequest();
 55+ $target = !is_null( $par ) ? $par : $request->getVal( 'target' );
5856
5957 // Yes, the use of getVal() and getText() is wanted, see bug 20365
60 - $oldTitleText = $wgRequest->getVal( 'wpOldTitle', $target );
61 - $newTitleText = $wgRequest->getText( 'wpNewTitle' );
 58+ $oldTitleText = $request->getVal( 'wpOldTitle', $target );
 59+ $newTitleText = $request->getText( 'wpNewTitle' );
6260
6361 $this->oldTitle = Title::newFromText( $oldTitleText );
6462 $this->newTitle = Title::newFromText( $newTitleText );
6563
6664 if( is_null( $this->oldTitle ) ) {
67 - $wgOut->showErrorPage( 'notargettitle', 'notargettext' );
68 - return;
 65+ throw new ErrorPageError( 'notargettitle', 'notargettext' );
6966 }
7067 if( !$this->oldTitle->exists() ) {
71 - $wgOut->showErrorPage( 'nopagetitle', 'nopagetext' );
72 - return;
 68+ throw new ErrorPageError( 'nopagetitle', 'nopagetext' );
7369 }
7470
 71+ $user = $this->getUser();
 72+
7573 # Check rights
76 - $permErrors = $this->oldTitle->getUserPermissionsErrors( 'move', $wgUser );
 74+ $permErrors = $this->oldTitle->getUserPermissionsErrors( 'move', $user );
7775 if( !empty( $permErrors ) ) {
78 - $wgOut->showPermissionsErrorPage( $permErrors );
 76+ $this->getOutput()->showPermissionsErrorPage( $permErrors );
7977 return;
8078 }
8179
82 - $def = !$wgRequest->wasPosted();
 80+ $def = !$request->wasPosted();
8381
84 - $this->reason = $wgRequest->getText( 'wpReason' );
85 - $this->moveTalk = $wgRequest->getBool( 'wpMovetalk', $def );
86 - $this->fixRedirects = $wgRequest->getBool( 'wpFixRedirects', $def );
87 - $this->leaveRedirect = $wgRequest->getBool( 'wpLeaveRedirect', $def );
88 - $this->moveSubpages = $wgRequest->getBool( 'wpMovesubpages', false );
89 - $this->deleteAndMove = $wgRequest->getBool( 'wpDeleteAndMove' ) && $wgRequest->getBool( 'wpConfirm' );
90 - $this->moveOverShared = $wgRequest->getBool( 'wpMoveOverSharedFile', false );
91 - $this->watch = $wgRequest->getCheck( 'wpWatch' ) && $wgUser->isLoggedIn();
 82+ $this->reason = $request->getText( 'wpReason' );
 83+ $this->moveTalk = $request->getBool( 'wpMovetalk', $def );
 84+ $this->fixRedirects = $request->getBool( 'wpFixRedirects', $def );
 85+ $this->leaveRedirect = $request->getBool( 'wpLeaveRedirect', $def );
 86+ $this->moveSubpages = $request->getBool( 'wpMovesubpages', false );
 87+ $this->deleteAndMove = $request->getBool( 'wpDeleteAndMove' ) && $request->getBool( 'wpConfirm' );
 88+ $this->moveOverShared = $request->getBool( 'wpMoveOverSharedFile', false );
 89+ $this->watch = $request->getCheck( 'wpWatch' ) && $user->isLoggedIn();
9290
93 - if ( 'submit' == $wgRequest->getVal( 'action' ) && $wgRequest->wasPosted()
94 - && $wgUser->matchEditToken( $wgRequest->getVal( 'wpEditToken' ) ) ) {
 91+ if ( 'submit' == $request->getVal( 'action' ) && $request->wasPosted()
 92+ && $user->matchEditToken( $request->getVal( 'wpEditToken' ) ) ) {
9593 $this->doSubmit();
9694 } else {
9795 $this->showForm( '' );
——@@ -105,17 +103,16 @@
106104 * OutputPage::wrapWikiMsg().
107105 */
108106 function showForm( $err ) {
109 - global $wgOut, $wgUser, $wgContLang, $wgFixDoubleRedirects;
 107+ global $wgContLang, $wgFixDoubleRedirects, $wgMaximumMovedPages;
110108
111 - $skin = $this->getSkin();
 109+ $this->getSkin()->setRelevantTitle( $this->oldTitle );
112110
113 - $oldTitleLink = $skin->link( $this->oldTitle );
 111+ $oldTitleLink = Linker::link( $this->oldTitle );
114112
115 - $wgOut->setPagetitle( wfMsg( 'move-page', $this->oldTitle->getPrefixedText() ) );
116 - $skin->setRelevantTitle( $this->oldTitle );
 113+ $out = $this->getOutput();
 114+ $out->setPagetitle( wfMsg( 'move-page', $this->oldTitle->getPrefixedText() ) );
 115+ $out->addModules( 'mediawiki.special.movePage' );
117116
118 - $wgOut->addModules( 'mediawiki.special.movePage' );
119 -
120117 $newTitle = $this->newTitle;
121118
122119 if( !$newTitle ) {
——@@ -135,8 +132,10 @@
136133 }
137134 }
138135
139 - if ( !empty($err) && $err[0] == 'articleexists' && $wgUser->isAllowed( 'delete' ) ) {
140 - $wgOut->addWikiMsg( 'delete_and_move_text', $newTitle->getPrefixedText() );
 136+ $user = $this->getUser();
 137+
 138+ if ( !empty($err) && $err[0] == 'articleexists' && $user->isAllowed( 'delete' ) ) {
 139+ $out->addWikiMsg( 'delete_and_move_text', $newTitle->getPrefixedText() );
141140 $movepagebtn = wfMsg( 'delete_and_move' );
142141 $submitVar = 'wpDeleteAndMove';
143142 $confirm = "
——@@ -149,17 +148,17 @@
150149 $err = '';
151150 } else {
152151 if ($this->oldTitle->getNamespace() == NS_USER && !$this->oldTitle->isSubpage() ) {
153 - $wgOut->wrapWikiMsg( "<div class=\"error mw-moveuserpage-warning\">\n$1\n</div>", 'moveuserpage-warning' );
 152+ $out->wrapWikiMsg( "<div class=\"error mw-moveuserpage-warning\">\n$1\n</div>", 'moveuserpage-warning' );
154153 }
155 - $wgOut->addWikiMsg( $wgFixDoubleRedirects ? 'movepagetext' :
 154+ $out->addWikiMsg( $wgFixDoubleRedirects ? 'movepagetext' :
156155 'movepagetext-noredirectfixer' );
157156 $movepagebtn = wfMsg( 'movepagebtn' );
158157 $submitVar = 'wpMove';
159158 $confirm = false;
160159 }
161160
162 - if ( !empty($err) && $err[0] == 'file-exists-sharedrepo' && $wgUser->isAllowed( 'reupload-shared' ) ) {
163 - $wgOut->addWikiMsg( 'move-over-sharedrepo', $newTitle->getPrefixedText() );
 161+ if ( !empty($err) && $err[0] == 'file-exists-sharedrepo' && $user->isAllowed( 'reupload-shared' ) ) {
 162+ $out->addWikiMsg( 'move-over-sharedrepo', $newTitle->getPrefixedText() );
164163 $submitVar = 'wpMoveOverSharedFile';
165164 $err = '';
166165 }
——@@ -179,19 +178,19 @@
180179 }
181180
182181 if ( $considerTalk ) {
183 - $wgOut->addWikiMsg( 'movepagetalktext' );
 182+ $out->addWikiMsg( 'movepagetalktext' );
184183 }
185184
186 - $token = htmlspecialchars( $wgUser->editToken() );
 185+ $token = htmlspecialchars( $user->editToken() );
187186
188187 if ( !empty($err) ) {
189 - $wgOut->setSubtitle( wfMsg( 'formerror' ) );
 188+ $out->setSubtitle( wfMsg( 'formerror' ) );
190189 if( $err[0] == 'hookaborted' ) {
191190 $hookErr = $err[1];
192191 $errMsg = "<p><strong class=\"error\">$hookErr</strong></p>\n";
193 - $wgOut->addHTML( $errMsg );
 192+ $out->addHTML( $errMsg );
194193 } else {
195 - $wgOut->wrapWikiMsg( "<p><strong class=\"error\">\n$1\n</strong></p>", $err );
 194+ $out->wrapWikiMsg( "<p><strong class=\"error\">\n$1\n</strong></p>", $err );
196195 }
197196 }
198197
——@@ -205,13 +204,13 @@
206205 $noticeMsg = 'protectedpagemovewarning';
207206 $classes[] = 'mw-textarea-protected';
208207 }
209 - $wgOut->addHTML( "<div class='mw-warning-with-logexcerpt'>\n" );
210 - $wgOut->addWikiMsg( $noticeMsg );
211 - LogEventsList::showLogExtract( $wgOut, 'protect', $this->oldTitle->getPrefixedText(), '', array( 'lim' => 1 ) );
212 - $wgOut->addHTML( "</div>\n" );
 208+ $out->addHTML( "<div class='mw-warning-with-logexcerpt'>\n" );
 209+ $out->addWikiMsg( $noticeMsg );
 210+ LogEventsList::showLogExtract( $out, 'protect', $this->oldTitle->getPrefixedText(), '', array( 'lim' => 1 ) );
 211+ $out->addHTML( "</div>\n" );
213212 }
214213
215 - $wgOut->addHTML(
 214+ $out->addHTML(
216215 Xml::openElement( 'form', array( 'method' => 'post', 'action' => $this->getTitle()->getLocalURL( 'action=submit' ), 'id' => 'movepage' ) ) .
217216 Xml::openElement( 'fieldset' ) .
218217 Xml::element( 'legend', null, wfMsg( 'move-page-legend' ) ) .
——@@ -245,7 +244,7 @@
246245 );
247246
248247 if( $considerTalk ) {
249 - $wgOut->addHTML( "
 248+ $out->addHTML( "
250249 <tr>
251250 <td></td>
252251 <td class='mw-input'>" .
——@@ -255,8 +254,8 @@
256255 );
257256 }
258257
259 - if ( $wgUser->isAllowed( 'suppressredirect' ) ) {
260 - $wgOut->addHTML( "
 258+ if ( $user->isAllowed( 'suppressredirect' ) ) {
 259+ $out->addHTML( "
261260 <tr>
262261 <td></td>
263262 <td class='mw-input' >" .
——@@ -268,7 +267,7 @@
269268 }
270269
271270 if ( $hasRedirects ) {
272 - $wgOut->addHTML( "
 271+ $out->addHTML( "
273272 <tr>
274273 <td></td>
275274 <td class='mw-input' >" .
——@@ -280,11 +279,9 @@
281280 }
282281
283282 if( ($this->oldTitle->hasSubpages() || $this->oldTitle->getTalkPage()->hasSubpages())
284 - && $this->oldTitle->userCan( 'move-subpages' ) )
 283+ && !count( $this->oldTitle->getUserPermissionsErrors( 'move-subpages', $user ) ) )
285284 {
286 - global $wgMaximumMovedPages, $wgLang;
287 -
288 - $wgOut->addHTML( "
 285+ $out->addHTML( "
289286 <tr>
290287 <td></td>
291288 <td class=\"mw-input\">" .
——@@ -301,7 +298,7 @@
302299 ? 'move-subpages'
303300 : 'move-talk-subpages' ),
304301 array( 'parseinline' ),
305 - $wgLang->formatNum( $wgMaximumMovedPages ),
 302+ $this->getLang()->formatNum( $wgMaximumMovedPages ),
306303 # $2 to allow use of PLURAL in message.
307304 $wgMaximumMovedPages
308305 )
——@@ -311,11 +308,11 @@
312309 );
313310 }
314311
315 - $watchChecked = $wgUser->isLoggedIn() && ($this->watch || $wgUser->getBoolOption( 'watchmoves' )
 312+ $watchChecked = $user->isLoggedIn() && ($this->watch || $user->getBoolOption( 'watchmoves' )
316313 || $this->oldTitle->userIsWatching());
317314 # Don't allow watching if user is not logged in
318 - if( $wgUser->isLoggedIn() ) {
319 - $wgOut->addHTML( "
 315+ if( $user->isLoggedIn() ) {
 316+ $out->addHTML( "
320317 <tr>
321318 <td></td>
322319 <td class='mw-input'>" .
——@@ -324,7 +321,7 @@
325322 </tr>");
326323 }
327324
328 - $wgOut->addHTML( "
 325+ $out->addHTML( "
329326 {$confirm}
330327 <tr>
331328 <td>&#160;</td>
——@@ -339,32 +336,31 @@
340337 "\n"
341338 );
342339
343 - $this->showLogFragment( $this->oldTitle, $wgOut );
344 - $this->showSubpages( $this->oldTitle, $wgOut );
 340+ $this->showLogFragment( $this->oldTitle );
 341+ $this->showSubpages( $this->oldTitle );
345342
346343 }
347344
348345 function doSubmit() {
349 - global $wgOut, $wgUser, $wgMaximumMovedPages, $wgLang;
350 - global $wgFixDoubleRedirects;
 346+ global $wgMaximumMovedPages, $wgFixDoubleRedirects, $wgDeleteRevisionsLimit;
351347
352 - if ( $wgUser->pingLimiter( 'move' ) ) {
353 - $wgOut->rateLimited();
354 - return;
 348+ $user = $this->getUser();
 349+
 350+ if ( $user->pingLimiter( 'move' ) ) {
 351+ throw new ThrottledError;
355352 }
356353
357354 $ot = $this->oldTitle;
358355 $nt = $this->newTitle;
359356
360357 # Delete to make way if requested
361 - if ( $wgUser->isAllowed( 'delete' ) && $this->deleteAndMove ) {
 358+ if ( $user->isAllowed( 'delete' ) && $this->deleteAndMove ) {
362359 $article = new Article( $nt );
363360
364361 # Disallow deletions of big articles
365362 $bigHistory = $article->isBigDeletion();
366 - if( $bigHistory && !$nt->userCan( 'bigdelete' ) ) {
367 - global $wgDeleteRevisionsLimit;
368 - $this->showForm( array('delete-toobig', $wgLang->formatNum( $wgDeleteRevisionsLimit ) ) );
 363+ if( $bigHistory && count( $nt->getUserPermissionsErrors( 'bigdelete', $user ) ) ) {
 364+ $this->showForm( array('delete-toobig', $this->getLang()->formatNum( $wgDeleteRevisionsLimit ) ) );
369365 return;
370366 }
371367
——@@ -386,7 +382,7 @@
387383
388384 # Show a warning if the target file exists on a shared repo
389385 if ( $nt->getNamespace() == NS_FILE
390 - && !( $this->moveOverShared && $wgUser->isAllowed( 'reupload-shared' ) )
 386+ && !( $this->moveOverShared && $user->isAllowed( 'reupload-shared' ) )
391387 && !RepoGroup::singleton()->getLocalRepo()->findFile( $nt )
392388 && wfFindFile( $nt ) )
393389 {
——@@ -395,7 +391,7 @@
396392
397393 }
398394
399 - if ( $wgUser->isAllowed( 'suppressredirect' ) ) {
 395+ if ( $user->isAllowed( 'suppressredirect' ) ) {
400396 $createRedirect = $this->leaveRedirect;
401397 } else {
402398 $createRedirect = true;
——@@ -415,7 +411,8 @@
416412
417413 wfRunHooks( 'SpecialMovepageAfterMove', array( &$this, &$ot, &$nt ) );
418414
419 - $wgOut->setPagetitle( wfMsg( 'pagemovedsub' ) );
 415+ $out = $this->getOutput();
 416+ $out->setPagetitle( wfMsg( 'pagemovedsub' ) );
420417
421418 $oldUrl = $ot->getFullUrl( 'redirect=no' );
422419 $newUrl = $nt->getFullUrl();
——@@ -425,8 +422,8 @@
426423 $newLink = "<span class='plainlinks'>[$newUrl $newText]</span>";
427424
428425 $msgName = $createRedirect ? 'movepage-moved-redirect' : 'movepage-moved-noredirect';
429 - $wgOut->addWikiMsg( 'movepage-moved', $oldLink, $newLink, $oldText, $newText );
430 - $wgOut->addWikiMsg( $msgName );
 426+ $out->addWikiMsg( 'movepage-moved', $oldLink, $newLink, $oldText, $newText );
 427+ $out->addWikiMsg( $msgName );
431428
432429 # Now we move extra pages we've been asked to move: subpages and talk
433430 # pages. First, if the old page or the new page is a talk page, we
——@@ -435,7 +432,7 @@
436433 $this->moveTalk = false;
437434 }
438435
439 - if( !$ot->userCan( 'move-subpages' ) ) {
 436+ if ( count( $ot->getUserPermissionsErrors( 'move-subpages', $user ) ) ) {
440437 $this->moveSubpages = false;
441438 }
442439
——@@ -492,7 +489,6 @@
493490 }
494491
495492 $extraOutput = array();
496 - $skin = $this->getSkin();
497493 $count = 1;
498494 foreach( $extraPages as $oldSubpage ) {
499495 if( $ot->equals( $oldSubpage ) ) {
——@@ -514,7 +510,7 @@
515511 # be longer than 255 characters.
516512 $newSubpage = Title::makeTitleSafe( $newNs, $newPageName );
517513 if( !$newSubpage ) {
518 - $oldLink = $skin->linkKnown( $oldSubpage );
 514+ $oldLink = Linker::linkKnown( $oldSubpage );
519515 $extraOutput []= wfMsgHtml( 'movepage-page-unmoved', $oldLink,
520516 htmlspecialchars(Title::makeName( $newNs, $newPageName )));
521517 continue;
——@@ -522,7 +518,7 @@
523519
524520 # This was copy-pasted from Renameuser, bleh.
525521 if ( $newSubpage->exists() && !$oldSubpage->isValidMoveTarget( $newSubpage ) ) {
526 - $link = $skin->linkKnown( $newSubpage );
 522+ $link = Linker::linkKnown( $newSubpage );
527523 $extraOutput []= wfMsgHtml( 'movepage-page-exists', $link );
528524 } else {
529525 $success = $oldSubpage->moveTo( $newSubpage, true, $this->reason, $createRedirect );
——@@ -530,22 +526,22 @@
531527 if ( $this->fixRedirects ) {
532528 DoubleRedirectJob::fixRedirects( 'move', $oldSubpage, $newSubpage );
533529 }
534 - $oldLink = $skin->linkKnown(
 530+ $oldLink = Linker::linkKnown(
535531 $oldSubpage,
536532 null,
537533 array(),
538534 array( 'redirect' => 'no' )
539535 );
540 - $newLink = $skin->linkKnown( $newSubpage );
 536+ $newLink = Linker::linkKnown( $newSubpage );
541537 $extraOutput []= wfMsgHtml( 'movepage-page-moved', $oldLink, $newLink );
542538 ++$count;
543539 if( $count >= $wgMaximumMovedPages ) {
544 - $extraOutput []= wfMsgExt( 'movepage-max-pages', array( 'parsemag', 'escape' ), $wgLang->formatNum( $wgMaximumMovedPages ) );
 540+ $extraOutput []= wfMsgExt( 'movepage-max-pages', array( 'parsemag', 'escape' ), $this->getLang()->formatNum( $wgMaximumMovedPages ) );
545541 break;
546542 }
547543 } else {
548 - $oldLink = $skin->linkKnown( $oldSubpage );
549 - $newLink = $skin->link( $newSubpage );
 544+ $oldLink = Linker::linkKnown( $oldSubpage );
 545+ $newLink = Linker::link( $newSubpage );
550546 $extraOutput []= wfMsgHtml( 'movepage-page-unmoved', $oldLink, $newLink );
551547 }
552548 }
——@@ -553,16 +549,16 @@
554550 }
555551
556552 if( $extraOutput !== array() ) {
557 - $wgOut->addHTML( "<ul>\n<li>" . implode( "</li>\n<li>", $extraOutput ) . "</li>\n</ul>" );
 553+ $out->addHTML( "<ul>\n<li>" . implode( "</li>\n<li>", $extraOutput ) . "</li>\n</ul>" );
558554 }
559555
560556 # Deal with watches (we don't watch subpages)
561 - if( $this->watch && $wgUser->isLoggedIn() ) {
562 - $wgUser->addWatch( $ot );
563 - $wgUser->addWatch( $nt );
 557+ if( $this->watch && $user->isLoggedIn() ) {
 558+ $user->addWatch( $ot );
 559+ $user->addWatch( $nt );
564560 } else {
565 - $wgUser->removeWatch( $ot );
566 - $wgUser->removeWatch( $nt );
 561+ $user->removeWatch( $ot );
 562+ $user->removeWatch( $nt );
567563 }
568564
569565 # Re-clear the file redirect cache, which may have been polluted by
——@@ -573,20 +569,20 @@
574570 }
575571 }
576572
577 - function showLogFragment( $title, &$out ) {
 573+ function showLogFragment( $title ) {
 574+ $out = $this->getOutput();
578575 $out->addHTML( Xml::element( 'h2', null, LogPage::logName( 'move' ) ) );
579576 LogEventsList::showLogExtract( $out, 'move', $title->getPrefixedText() );
580577 }
581578
582 - function showSubpages( $title, $out ) {
583 - global $wgLang;
584 -
 579+ function showSubpages( $title ) {
585580 if( !MWNamespace::hasSubpages( $title->getNamespace() ) )
586581 return;
587582
588583 $subpages = $title->getSubpages();
589584 $count = $subpages instanceof TitleArray ? $subpages->count() : 0;
590585
 586+ $out = $this->getOutput();
591587 $out->wrapWikiMsg( '== $1 ==', array( 'movesubpage', $count ) );
592588
593589 # No subpages.
——@@ -595,12 +591,11 @@
596592 return;
597593 }
598594
599 - $out->addWikiMsg( 'movesubpagetext', $wgLang->formatNum( $count ) );
600 - $skin = $this->getSkin();
 595+ $out->addWikiMsg( 'movesubpagetext', $this->getLang()->formatNum( $count ) );
601596 $out->addHTML( "<ul>\n" );
602597
603598 foreach( $subpages as $subpage ) {
604 - $link = $skin->link( $subpage );
 599+ $link = Linker::link( $subpage );
605600 $out->addHTML( "<li>$link</li>\n" );
606601 }
607602 $out->addHTML( "</ul>\n" );

Status & tagging log