r70720 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r70719‎ | r70720 | r70721 >
Date:18:46, 8 August 2010
Author:ialex
Status:ok (Comments)
Tags:
Comment:
Modified Special:Movepage to subclass UnlistedSpecialPage
Modified paths:
  • /trunk/extensions/Translate/tag/SpecialPageTranslationMovePage.php (modified) (history)
  • /trunk/phase3/includes/SpecialPage.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialMovepage.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/specials/SpecialMovepage.php
@@ -18,87 +18,74 @@
1919 */
2020
2121 /**
22 - * @file
 22+ * Implements Special:Movepage
2323 * @ingroup SpecialPage
2424 */
 25+class MovePageForm extends UnlistedSpecialPage {
 26+ var $oldTitle, $newTitle; # Objects
 27+ var $reason; # Text input
 28+ var $moveTalk, $deleteAndMove, $moveSubpages, $fixRedirects, $leaveRedirect, $moveOverShared; # Checks
2529
26 -/**
27 - * Constructor
28 - */
29 -function wfSpecialMovepage( $par = null ) {
30 - global $wgUser, $wgOut, $wgRequest;
 30+ private $watch = false;
3131
32 - # Check for database lock
33 - if ( wfReadOnly() ) {
34 - $wgOut->readOnlyPage();
35 - return;
 32+ public function __construct() {
 33+ parent::__construct( 'Movepage' );
3634 }
3735
38 - $target = isset( $par ) ? $par : $wgRequest->getVal( 'target' );
 36+ public function execute( $par ) {
 37+ global $wgUser, $wgOut, $wgRequest;
3938
40 - // Yes, the use of getVal() and getText() is wanted, see bug 20365
41 - $oldTitleText = $wgRequest->getVal( 'wpOldTitle', $target );
42 - $newTitleText = $wgRequest->getText( 'wpNewTitle' );
 39+ # Check for database lock
 40+ if ( wfReadOnly() ) {
 41+ $wgOut->readOnlyPage();
 42+ return;
 43+ }
4344
44 - $oldTitle = Title::newFromText( $oldTitleText );
45 - $newTitle = Title::newFromText( $newTitleText );
 45+ $this->setHeaders();
 46+ $this->outputHeader();
4647
47 - if( is_null( $oldTitle ) ) {
48 - $wgOut->showErrorPage( 'notargettitle', 'notargettext' );
49 - return;
50 - }
51 - if( !$oldTitle->exists() ) {
52 - $wgOut->showErrorPage( 'nopagetitle', 'nopagetext' );
53 - return;
54 - }
 48+ $target = !is_null( $par ) ? $par : $wgRequest->getVal( 'target' );
5549
56 - # Check rights
57 - $permErrors = $oldTitle->getUserPermissionsErrors( 'move', $wgUser );
58 - if( !empty( $permErrors ) ) {
59 - $wgOut->showPermissionsErrorPage( $permErrors );
60 - return;
61 - }
 50+ // Yes, the use of getVal() and getText() is wanted, see bug 20365
 51+ $oldTitleText = $wgRequest->getVal( 'wpOldTitle', $target );
 52+ $newTitleText = $wgRequest->getText( 'wpNewTitle' );
6253
63 - $form = new MovePageForm( $oldTitle, $newTitle );
 54+ $this->oldTitle = Title::newFromText( $oldTitleText );
 55+ $this->newTitle = Title::newFromText( $newTitleText );
6456
65 - if ( 'submit' == $wgRequest->getVal( 'action' ) && $wgRequest->wasPosted()
66 - && $wgUser->matchEditToken( $wgRequest->getVal( 'wpEditToken' ) ) ) {
67 - $form->doSubmit();
68 - } else {
69 - $form->showForm( '' );
70 - }
71 -}
 57+ if( is_null( $this->oldTitle ) ) {
 58+ $wgOut->showErrorPage( 'notargettitle', 'notargettext' );
 59+ return;
 60+ }
 61+ if( !$this->oldTitle->exists() ) {
 62+ $wgOut->showErrorPage( 'nopagetitle', 'nopagetext' );
 63+ return;
 64+ }
7265
73 -/**
74 - * HTML form for Special:Movepage
75 - * @ingroup SpecialPage
76 - */
77 -class MovePageForm {
78 - var $oldTitle, $newTitle; # Objects
79 - var $reason; # Text input
80 - var $moveTalk, $deleteAndMove, $moveSubpages, $fixRedirects, $leaveRedirect, $moveOverShared; # Checks
 66+ # Check rights
 67+ $permErrors = $this->oldTitle->getUserPermissionsErrors( 'move', $wgUser );
 68+ if( !empty( $permErrors ) ) {
 69+ $wgOut->showPermissionsErrorPage( $permErrors );
 70+ return;
 71+ }
8172
82 - private $watch = false;
 73+ $def = !$wgRequest->wasPosted();
8374
84 - function __construct( $oldTitle, $newTitle ) {
85 - global $wgRequest, $wgUser;
86 -
87 - $this->oldTitle = $oldTitle;
88 - $this->newTitle = $newTitle;
8975 $this->reason = $wgRequest->getText( 'wpReason' );
90 - if ( $wgRequest->wasPosted() ) {
91 - $this->moveTalk = $wgRequest->getBool( 'wpMovetalk', false );
92 - $this->fixRedirects = $wgRequest->getBool( 'wpFixRedirects', false );
93 - $this->leaveRedirect = $wgRequest->getBool( 'wpLeaveRedirect', false );
94 - } else {
95 - $this->moveTalk = $wgRequest->getBool( 'wpMovetalk', true );
96 - $this->fixRedirects = $wgRequest->getBool( 'wpFixRedirects', true );
97 - $this->leaveRedirect = $wgRequest->getBool( 'wpLeaveRedirect', true );
98 - }
 76+ $this->moveTalk = $wgRequest->getBool( 'wpMovetalk', $def );
 77+ $this->fixRedirects = $wgRequest->getBool( 'wpFixRedirects', $def );
 78+ $this->leaveRedirect = $wgRequest->getBool( 'wpLeaveRedirect', $def );
9979 $this->moveSubpages = $wgRequest->getBool( 'wpMovesubpages', false );
10080 $this->deleteAndMove = $wgRequest->getBool( 'wpDeleteAndMove' ) && $wgRequest->getBool( 'wpConfirm' );
10181 $this->moveOverShared = $wgRequest->getBool( 'wpMoveOverSharedFile', false );
10282 $this->watch = $wgRequest->getCheck( 'wpWatch' ) && $wgUser->isLoggedIn();
 83+
 84+ if ( 'submit' == $wgRequest->getVal( 'action' ) && $wgRequest->wasPosted()
 85+ && $wgUser->matchEditToken( $wgRequest->getVal( 'wpEditToken' ) ) ) {
 86+ $this->doSubmit();
 87+ } else {
 88+ $this->showForm( '' );
 89+ }
10390 }
10491
10592 /**
@@ -183,7 +170,6 @@
184171 $wgOut->addWikiMsg( 'movepagetalktext' );
185172 }
186173
187 - $titleObj = SpecialPage::getTitleFor( 'Movepage' );
188174 $token = htmlspecialchars( $wgUser->editToken() );
189175
190176 if ( !empty($err) ) {
@@ -214,7 +200,7 @@
215201 }
216202
217203 $wgOut->addHTML(
218 - Xml::openElement( 'form', array( 'method' => 'post', 'action' => $titleObj->getLocalURL( 'action=submit' ), 'id' => 'movepage' ) ) .
 204+ Xml::openElement( 'form', array( 'method' => 'post', 'action' => $this->getTitle()->getLocalURL( 'action=submit' ), 'id' => 'movepage' ) ) .
219205 Xml::openElement( 'fieldset' ) .
220206 Xml::element( 'legend', null, wfMsg( 'move-page-legend' ) ) .
221207 Xml::openElement( 'table', array( 'border' => '0', 'id' => 'mw-movepage-table' ) ) .
Index: trunk/phase3/includes/SpecialPage.php
@@ -185,7 +185,7 @@
186186 'Blankpage' => 'SpecialBlankpage',
187187 'Blockme' => 'SpecialBlockme',
188188 'Emailuser' => 'SpecialEmailUser',
189 - 'Movepage' => array( 'UnlistedSpecialPage', 'Movepage' ),
 189+ 'Movepage' => 'MovePageForm',
190190 'Mycontributions' => 'SpecialMycontributions',
191191 'Mypage' => 'SpecialMypage',
192192 'Mytalk' => 'SpecialMytalk',
Index: trunk/extensions/Translate/tag/SpecialPageTranslationMovePage.php
@@ -103,7 +103,7 @@
104104
105105 } else {
106106 // Delegate... don't want to reimplement this
107 - $this->doNormalMovePage();
 107+ $this->doNormalMovePage( $par );
108108 }
109109 }
110110
@@ -140,14 +140,9 @@
141141 return true;
142142 }
143143
144 - protected function doNormalMovePage() {
145 - global $wgRequest;
146 - $form = new MovePageForm( $this->oldTitle, $this->newTitle );
147 - if ( 'submit' == $wgRequest->getVal( 'action' ) && $this->checkToken() && $wgRequest->wasPosted() ) {
148 - $form->doSubmit();
149 - } else {
150 - $form->showForm( '' );
151 - }
 144+ protected function doNormalMovePage( $par ) {
 145+ $sp = new MovePageForm();
 146+ $sp->execute( $par );
152147 }
153148
154149 /**

Follow-up revisions

RevisionCommit summaryAuthorDate
r72403Per Nikerabbit, fix for r70720: make Translate compatible with 1.16ialex09:22, 5 September 2010

Comments

#Comment by Nikerabbit (talk | contribs)   12:21, 30 August 2010

Translate extension should be compatible with 1.16.

#Comment by Reedy (talk | contribs)   02:26, 6 January 2011

Seems to work fine. Issues have been resolved and marked ok on r72403

Status & tagging log