r45102 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r45101‎ | r45102 | r45103 >
Date:20:02, 27 December 2008
Author:mrzman
Status:deferred (Comments)
Tags:
Comment:
more Special:Interwiki cleanup:
* standardize the error message display
* show the form after an edit/delete error and preload the fields with the previous input
* quick filter for bad characters
Modified paths:
  • /trunk/phase3/includes/specials/SpecialInterwiki.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
——@@ -1471,6 +1471,7 @@
14721472 'interwiki_addfailed',
14731473 'interwiki_addintro',
14741474 'interwiki_addtext',
 1475+ 'interwiki-badprefix',
14751476 'interwiki_defaultreason',
14761477 'interwiki_defaulturl',
14771478 'interwiki_deleted',
Index: trunk/phase3/includes/specials/SpecialInterwiki.php
——@@ -56,7 +56,7 @@
5757
5858 $actionUrl = $this->getTitle()->getLocalURL( 'action=submit' );
5959 $token = $wgUser->editToken();
60 - $defaultreason = wfMsgForContent( 'interwiki_defaultreason' );
 60+ $defaultreason = $wgRequest->getVal( 'wpInterwikiReason' ) ? $wgRequest->getVal( 'wpInterwikiReason' ) : wfMsgForContent( 'interwiki_defaultreason' );
6161
6262 switch( $action ){
6363 case "delete":
——@@ -94,7 +94,7 @@
9595 $dbr = wfGetDB( DB_SLAVE );
9696 $row = $dbr->selectRow( 'interwiki', '*', array( 'iw_prefix' => $prefix ) );
9797 if( !$row ){
98 - $wgOut->wrapWikiMsg( '<div class="errorbox">$1</div>', array( 'interwiki_editerror', $prefix ) );
 98+ $this->error( wfMsg( 'interwiki_editerror', $prefix ) );
9999 return;
100100 }
101101 $prefix = '<tt>' . htmlspecialchars( $row->iw_prefix ) . '</tt>';
——@@ -106,11 +106,12 @@
107107 $intromessage = wfMsgExt( 'interwiki_editintro', array( 'parseinline' ) );
108108 $button = wfMsg( 'edit' );
109109 } else {
110 - $prefix = Xml::input( 'wpInterwikiPrefix', 20, false, array( 'tabindex'=>'1', 'id'=>'mw-interwiki-prefix', 'maxlength'=>'20' ) );
111 - $local = false;
112 - $trans = false;
 110+ $prefix = $wgRequest->getVal( 'wpInterwikiPrefix' ) ? $wgRequest->getVal( 'wpInterwikiPrefix' ) : $wgRequest->getVal( 'prefix' );
 111+ $prefix = Xml::input( 'wpInterwikiPrefix', 20, $prefix, array( 'tabindex'=>'1', 'id'=>'mw-interwiki-prefix', 'maxlength'=>'20' ) );
 112+ $local = $wgRequest->getCheck( 'wpInterwikiLocal' );
 113+ $trans = $wgRequest->getCheck( 'wpInterwikiTrans' );
113114 $old = '';
114 - $defaulturl = wfMsg( 'interwiki_defaulturl' );
 115+ $defaulturl = $wgRequest->getVal( 'wpInterwikiURL' ) ? $wgRequest->getVal( 'wpInterwikiURL' ) : wfMsg( 'interwiki_defaulturl' );
115116 $topmessage = wfMsgExt( 'interwiki_addtext', array( 'parseinline' ) );
116117 $intromessage = wfMsgExt( 'interwiki_addintro', array( 'parseinline' ) );
117118 $button = wfMsg( 'interwiki_addbutton' );
——@@ -121,7 +122,6 @@
122123 $transmessage = wfMsg( 'interwiki_trans' );
123124 $reasonmessage = wfMsg( 'interwiki_reasonfield' );
124125 $urlmessage = wfMsg( 'interwiki_url' );
125 - $defaultreason = wfMsgForContent( 'interwiki_defaultreason' );
126126
127127 $wgOut->addHTML(
128128 Xml::openElement( 'fieldset' ) .
——@@ -154,8 +154,13 @@
155155 function doSubmit() {
156156 global $wgRequest, $wgOut;
157157 $prefix = $wgRequest->getVal( 'wpInterwikiPrefix' );
 158+ $do = $wgRequest->getVal( 'wpInterwikiAction' );
 159+ if( preg_match( '/[\s:&=]/', $prefix ) ) {
 160+ $this->error( wfMsg( 'interwiki-badprefix', $prefix ) );
 161+ $this->showForm( $do );
 162+ return;
 163+ }
158164 $reason = $wgRequest->getText( 'wpInterwikiReason' );
159 - $do = $wgRequest->getVal( 'wpInterwikiAction' );
160165 $selfTitle = $this->getTitle();
161166 $dbw = wfGetDB( DB_MASTER );
162167 switch( $do ){
——@@ -163,7 +168,8 @@
164169 $dbw->delete( 'interwiki', array( 'iw_prefix' => $prefix ), __METHOD__ );
165170
166171 if ( $dbw->affectedRows() == 0 ) {
167 - $wgOut->addWikiText( '<span class="error">' . wfMsg( 'interwiki_delfailed', $prefix ) . '</span>' );
 172+ $this->error( wfMsg( 'interwiki_delfailed', $prefix ) );
 173+ $this->showForm( $do );
168174 } else {
169175 $wgOut->addWikiText( wfMsg( 'interwiki_deleted', $prefix ));
170176 $wgOut->returnToMain( false, $selfTitle );
——@@ -186,7 +192,8 @@
187193 }
188194
189195 if( $dbw->affectedRows() == 0 ) {
190 - $wgOut->wrapWikiMsg( '<span class="error">$1</span>', array( "interwiki_{$do}failed", $prefix ) );
 196+ $this->error( wfMsg( "interwiki_{$do}failed", $prefix ) );
 197+ $this->showForm( $do );
191198 } else {
192199 $wgOut->addWikiMsg( "interwiki_{$do}ed", $prefix );
193200 $wgOut->returnToMain( false, $selfTitle );
——@@ -218,7 +225,7 @@
219226 $res = $dbr->select( 'interwiki', '*' );
220227 $numrows = $res->numRows();
221228 if ( $numrows == 0 ) {
222 - $wgOut->wrapWikiMsg( '<div class="errorbox">$1</div>', 'interwiki_error' );
 229+ $this->error( wfMsgWikiHtml( 'interwiki_error' ) );
223230 return;
224231 }
225232
——@@ -259,4 +266,9 @@
260267 $out .= "</table><br />";
261268 $wgOut->addHTML( $out );
262269 }
 270+
 271+ function error( $msg ) {
 272+ global $wgOut;
 273+ $wgOut->addHTML( Xml::element('p', array( 'class' => 'error' ), $msg ) );
 274+ }
263275 }
Index: trunk/phase3/languages/messages/MessagesEn.php
——@@ -2196,6 +2196,7 @@
21972197 'interwiki_addintro' => 'You are adding a new interwiki prefix.
21982198 Remember that it cannot contain spaces ( ), colons (:), ampersands (&), or equal signs (=).',
21992199 'interwiki_addtext' => 'Add an interwiki prefix',
 2200+'interwiki-badprefix' => '"$1" contains invalid characters',
22002201 'interwiki_defaultreason' => 'no reason given',
22012202 'interwiki_defaulturl' => 'http://www.example.com/$1', # only translate this message to other languages if you have to change it
22022203 'interwiki_deleted' => 'Prefix "$1" was successfully removed from the interwiki table.',

Follow-up revisions

RevisionCommit summaryAuthorDate
r45115cleanup to r45102 per Nikerabbit on CodeReviewmrzman03:14, 28 December 2008

Comments

#Comment by Nikerabbit (talk | contribs)   22:45, 27 December 2008 
+ $defaultreason = $wgRequest->getVal( 'wpInterwikiReason' ) ? $wgRequest->getVal( 'wpInterwikiReason' ) : wfMsgForContent( 'interwiki_defaultreason' );

Doesn't getVal support default values? 

-$wgOut->wrapWikiMsg( '<div class="errorbox">$1</div>', 'interwiki_error' );
+$this->error( wfMsgWikiHtml( 'interwiki_error' ) );

This is a bad idea. If this really needs a wrapper, pass only the message name and possible parameters to error() and call wrapWikiMsg there. 

+'interwiki-badprefix'      => '"$1" contains invalid characters',

Maybe reword as "Specified interwiki prefix "$1" contains invalid characters" ?

#Comment by Mr.Z-man (talk | contribs)   03:15, 28 December 2008

cleaned up in r45115

Status & tagging log