MediaWiki r92529 - Code Review

Jump to: navigation, search
Repository:MediaWiki
Revision:< r92528‎ | r92529 (on ViewVC)‎ | r92530 >
Date:12:45, 19 July 2011
Author:robin
Status:resolved (Comments)
Tags:
Comment:
* Use interwiki cache (per r92528)
* Use ResourceLoader for css
* Some code clean-up/improvements
This version is only compatible with 1.19 (r92528)
Modified paths:

Diff [purge]

Index: trunk/extensions/Interwiki/Interwiki_body.php
——@@ -14,12 +14,10 @@
1515
1616 /**
1717 * Different description will be shown on Special:SpecialPage depending on
18 - * whether the user has the 'interwiki' right or not.
 18+ * whether the user can modify the data.
1919 */
2020 function getDescription() {
21 - global $wgUser;
22 -
23 - return wfMsg( $wgUser->isAllowed( 'interwiki' ) ?
 21+ return wfMsg( $this->canModify() ?
2422 'interwiki' : 'interwiki-title-norights' );
2523 }
2624
——@@ -31,37 +29,26 @@
3230 public function execute( $par ) {
3331 global $wgRequest, $wgOut, $wgUser;
3432
35 - $admin = $wgUser->isAllowed( 'interwiki' );
36 -
3733 $this->setHeaders();
3834 $this->outputHeader();
3935
40 - $admin = $wgUser->isAllowed( 'interwiki' );
41 - $action = $wgRequest->getVal( 'action', $par );
 36+ $wgOut->addModules( 'SpecialInterwiki' );
 37+
 38+ $action = $par ? $par : $wgRequest->getVal( 'action', $par );
4239 $return = $this->getTitle();
4340
4441 switch( $action ) {
4542 case 'delete':
4643 case 'edit':
4744 case 'add':
48 - if( !$admin ) {
49 - // Check permissions
50 - $wgOut->permissionRequired( 'interwiki' );
51 - } elseif( wfReadOnly() ) {
52 - // Is the database in read-only mode?
53 - $wgOut->readOnlyPage();
54 - } else {
 45+ if( $this->canModify( $wgOut ) ) {
5546 $this->showForm( $action );
5647 }
5748 $wgOut->returnToMain( false, $return );
5849 break;
5950 case 'submit':
60 - if( !$admin ) {
61 - // Check permissions
62 - $wgOut->permissionRequired( 'interwiki' );
63 - } elseif( wfReadOnly() ) {
64 - // Is the database in read-only mode?
65 - $wgOut->readOnlyPage();
 51+ if( !$this->canModify( $wgOut ) ) {
 52+ # Error msg added by canModify()
6653 } elseif( !$wgRequest->wasPosted() || !$wgUser->matchEditToken( $wgRequest->getVal( 'wpEditToken' ) ) ) {
6754 // Prevent cross-site request forgeries
6855 $wgOut->addWikiMsg( 'sessionfailure' );
——@@ -71,16 +58,37 @@
7259 $wgOut->returnToMain( false, $return );
7360 break;
7461 default:
75 - $this->showList( $admin );
 62+ $this->showList();
7663 break;
7764 }
7865 }
7966
 67+ /**
 68+ * Returns boolean whether the user can modify the data.
 69+ * @param $out If $wgOut object given, it adds the respective error message.
 70+ * @return Boolean
 71+ */
 72+ public function canModify( $out = false ) {
 73+ global $wgUser, $wgInterwikiCache;
 74+ if( !$wgUser->isAllowed( 'interwiki' ) ) {
 75+ # Check permissions
 76+ if( $out ) { $out->permissionRequired( 'interwiki' ); }
 77+ return false;
 78+ } elseif( $wgInterwikiCache ) {
 79+ # Editing the interwiki cache is not supported
 80+ if( $out ) { $out->addWikiMsg( 'interwiki-cached' ); }
 81+ return false;
 82+ } elseif( wfReadOnly() ) {
 83+ # Is the database in read-only mode?
 84+ if( $out ) { $out->readOnlyPage(); }
 85+ return false;
 86+ }
 87+ return true;
 88+ }
 89+
8090 function showForm( $action ) {
81 - global $wgRequest, $wgUser, $wgOut, $wgScriptPath;
 91+ global $wgRequest, $wgUser, $wgOut;
8292
83 - $wgOut->addExtensionStyle( "{$wgScriptPath}/extensions/Interwiki/Interwiki.css" );
84 -
8593 $actionUrl = $this->getTitle()->getLocalURL( 'action=submit' );
8694 $token = $wgUser->editToken();
8795
——@@ -238,103 +246,91 @@
239247 }
240248 }
241249
242 - function trans_local( $tl, $msg0, $msg1 ) {
243 - if( $tl === '0' ) {
244 - return $msg0;
245 - }
246 - if( $tl === '1' ) {
247 - return $msg1;
248 - }
249 - return htmlspecialchars( $tl );
250 - }
 250+ function showList() {
 251+ global $wgOut;
251252
252 - function showList( $admin ) {
253 - global $wgUser, $wgOut, $wgScriptPath;
 253+ $canModify = $this->canModify();
254254
255 - $wgOut->addExtensionStyle( "{$wgScriptPath}/extensions/Interwiki/Interwiki.css" );
256 -
257 - $prefixmessage = wfMsgHtml( 'interwiki_prefix' );
258 - $urlmessage = wfMsgHtml( 'interwiki_url' );
259 - $localmessage = wfMsgHtml( 'interwiki_local' );
260 - $transmessage = wfMsgHtml( 'interwiki_trans' );
261 - $message_0 = wfMsgHtml( 'interwiki_0' );
262 - $message_1 = wfMsgHtml( 'interwiki_1' );
263 - $alignStart = 'class="mw-align-'.wfUILang()->AlignStart().'"';
264 - $alignEnd = 'class="mw-align-'.wfUILang()->AlignEnd().'"';
265 -
266 - $out = '
267 -<table class="mw-interwikitable wikitable intro">
268 -<tr><th '.$alignStart.'>' . $prefixmessage . '</th><td>' . wfMsgExt( 'interwiki_prefix_intro', 'parseinline' ) . '</td></tr>
269 -<tr><th '.$alignStart.'>' . $urlmessage . '</th><td>' . wfMsgExt( 'interwiki_url_intro', 'parseinline' ) . '</td></tr>
270 -<tr><th '.$alignStart.'>' . $localmessage . '</th><td>' . wfMsgExt( 'interwiki_local_intro', 'parseinline' ) . '</td></tr>
271 -<tr><th '.$alignEnd.'>' . $message_0 . '</th><td>' . wfMsgExt( 'interwiki_local_0_intro', 'parseinline' ) . '</td></tr>
272 -<tr><th '.$alignEnd.'>' . $message_1 . '</th><td>' . wfMsgExt( 'interwiki_local_1_intro', 'parseinline' ) . '</td></tr>
273 -<tr><th '.$alignStart.'>' . $transmessage . '</th><td>' . wfMsgExt( 'interwiki_trans_intro', 'parseinline' ) . '</td></tr>
274 -<tr><th '.$alignEnd.'>' . $message_1 . '</th><td>' . wfMsgExt( 'interwiki_trans_1_intro', 'parseinline' ) . '</td></tr>
275 -<tr><th '.$alignEnd.'>' . $message_0 . '</th><td>' . wfMsgExt( 'interwiki_trans_0_intro', 'parseinline' ) . '</td></tr>
276 -</table>
277 -';
278255 $wgOut->addWikiMsg( 'interwiki_intro' );
279 - $wgOut->addHTML( $out );
 256+ $wgOut->addHTML(
 257+ Html::rawElement( 'table', array( 'class' => 'mw-interwikitable wikitable intro' ),
 258+ self::addInfoRow( 'start', 'interwiki_prefix', 'interwiki_prefix_intro' ) .
 259+ self::addInfoRow( 'start', 'interwiki_url', 'interwiki_url_intro' ) .
 260+ self::addInfoRow( 'start', 'interwiki_local', 'interwiki_local_intro' ) .
 261+ self::addInfoRow( 'end', 'interwiki_0', 'interwiki_local_0_intro' ) .
 262+ self::addInfoRow( 'end', 'interwiki_1', 'interwiki_local_1_intro' ) .
 263+ self::addInfoRow( 'start', 'interwiki_trans', 'interwiki_trans_intro' ) .
 264+ self::addInfoRow( 'end', 'interwiki_0', 'interwiki_local_0_intro' ) .
 265+ self::addInfoRow( 'end', 'interwiki_1', 'interwiki_local_1_intro' )
 266+ ) . "\n"
 267+ );
280268 $wgOut->addWikiMsg( 'interwiki_intro_footer' );
281269
282 - // Privileged users can add new prefixes
283 - if ( $admin ) {
284 - $skin = $wgUser->getSkin();
 270+ if ( $canModify ) {
285271 $addtext = wfMsgHtml( 'interwiki_addtext' );
286 - $addlink = $skin->link( $this->getTitle( 'add' ), $addtext );
 272+ $addlink = Linker::linkKnown( $this->getTitle( 'add' ), $addtext );
287273 $wgOut->addHTML( '<p class="mw-interwiki-addlink">' . $addlink . '</p>' );
288274 }
289275
290 - $dbr = wfGetDB( DB_SLAVE );
291 - $res = $dbr->select( 'interwiki', '*', false, __METHOD__ );
292 - $numrows = $res->numRows();
293 - if ( $numrows == 0 ) {
294 - // If the interwiki table is empty, display an error message
 276+ if( !method_exists( 'Interwiki', 'getAllPrefixes' ) ) {
 277+ # version 2.0 is not backwards compatible (but still display nice error)
295278 $this->error( 'interwiki_error' );
296279 return;
297280 }
 281+ $iwPrefixes = Interwiki::getAllPrefixes( null );
298282
299 - $selfTitle = $this->getTitle();
300 -
301 - $out = "
302 - <table class='mw-interwikitable wikitable sortable body'>
303 - <tr id='interwikitable-header'><th>$prefixmessage</th> <th>$urlmessage</th> <th>$localmessage</th> <th>$transmessage</th>";
304 - // Privileged users can modify and delete existing prefixes
305 - if( $admin ) {
306 - $deletemessage = wfMsgHtml( 'delete' );
307 - $editmessage = wfMsgHtml( 'edit' );
308 - $out .= '<th class="unsortable">' . wfMsgHtml( 'interwiki_edit' ) . '</th>';
 283+ if ( !is_array( $iwPrefixes ) || count( $iwPrefixes ) == 0 ) {
 284+ # If the interwiki table is empty, display an error message
 285+ $this->error( 'interwiki_error' );
 286+ return;
309287 }
310 - $out .= "</tr>\n";
311288
312 - while( $s = $res->fetchObject() ) {
313 - $prefix = htmlspecialchars( $s->iw_prefix );
314 - $url = htmlspecialchars( $s->iw_url );
315 - $trans = $this->trans_local( $s->iw_trans, $message_0, $message_1 );
316 - $local = $this->trans_local( $s->iw_local, $message_0, $message_1 );
317 - $out .= "<tr class='mw-interwikitable-row'>
318 - <td class='mw-interwikitable-prefix'>$prefix</td>
319 - <td class='mw-interwikitable-url'>$url</td>
320 - <td class='mw-interwikitable-local'>$local</td>
321 - <td class='mw-interwikitable-trans'>$trans</td>";
322 - if( $admin ) {
323 - $out .= '<td class="mw-interwikitable-modify">';
324 - $out .= $skin->link( $selfTitle, $editmessage, array(),
325 - array( 'action' => 'edit', 'prefix' => $s->iw_prefix ) );
326 - $out .= wfMsg( 'comma-separator' );
327 - $out .= $skin->link( $selfTitle, $deletemessage, array(),
328 - array( 'action' => 'delete', 'prefix' => $s->iw_prefix ) );
329 - $out .= '</td>';
330 - }
 289+ $out = '';
331290
332 - $out .= "\n</tr>\n";
 291+ # Output the table header
 292+ $out .= Html::openElement( 'table', array( 'class' => 'mw-interwikitable wikitable sortable body' ) ) . "\n";
 293+ $out .= Html::openElement( 'tr', array( 'id' => 'interwikitable-header' ) ) .
 294+ Html::element( 'th', null, wfMsgHtml( 'interwiki_prefix' ) ) .
 295+ Html::element( 'th', null, wfMsgHtml( 'interwiki_url' ) ) .
 296+ Html::element( 'th', null, wfMsgHtml( 'interwiki_local' ) ) .
 297+ Html::element( 'th', null, wfMsgHtml( 'interwiki_trans' ) ) .
 298+ ( $canModify ? Html::element( 'th', array( 'class' => 'unsortable' ), wfMsgHtml( 'interwiki_edit' ) ) : '' );
 299+ $out .= Html::closeElement( 'tr' ) . "\n";
 300+
 301+ $selfTitle = $this->getTitle();
 302+
 303+ foreach( $iwPrefixes as $i => $iwPrefix ) {
 304+ $out .= Html::openElement( 'tr', array( 'class' => 'mw-interwikitable-row' ) );
 305+ $out .= Html::element( 'td', array( 'class' => 'mw-interwikitable-prefix' ),
 306+ htmlspecialchars( $iwPrefix['iw_prefix'] ) );
 307+ $out .= Html::element( 'td', array( 'class' => 'mw-interwikitable-url' ), $iwPrefix['iw_url'] );
 308+ $out .= Html::element( 'td', array( 'class' => 'mw-interwikitable-local' ),
 309+ ( isset( $iwPrefix['iw_local'] ) ? wfMsgHtml( 'interwiki_' . $iwPrefix['iw_local'] ) : '-' ) );
 310+ $out .= Html::element( 'td', array( 'class' => 'mw-interwikitable-trans' ),
 311+ ( isset( $iwPrefix['iw_trans'] ) ? wfMsgHtml( 'interwiki_' . $iwPrefix['iw_trans'] ) : '-' ) );
 312+ if( $canModify ) {
 313+ $out .= Html::rawElement( 'td', array( 'class' => 'mw-interwikitable-modify' ),
 314+ Linker::linkKnown( $selfTitle, wfMsgHtml( 'edit' ), array(),
 315+ array( 'action' => 'edit', 'prefix' => $iwPrefix['iw_prefix'] ) ) .
 316+ wfMsg( 'comma-separator' ) .
 317+ Linker::linkKnown( $selfTitle, wfMsgHtml( 'delete' ), array(),
 318+ array( 'action' => 'delete', 'prefix' => $iwPrefix['iw_prefix'] ) )
 319+ );
 320+ }
 321+ $out .= Html::closeElement( 'tr' ) . "\n";
333322 }
334 - $res->free();
335 - $out .= '</table><br />';
 323+ $out .= Html::closeElement( 'table' );
 324+
336325 $wgOut->addHTML( $out );
337326 }
338327
 328+ static function addInfoRow( $align = 'start', $title, $text ) {
 329+ return Html::rawElement( 'tr', null,
 330+ Html::rawElement( 'th', array( 'class' => 'mw-align-' . $align ), wfMsg( $title ) ) .
 331+ Html::rawElement( 'td', null, wfMsgExt( $text, 'parseinline' ) )
 332+ );
 333+ }
 334+
339335 function error() {
340336 global $wgOut;
341337 $args = func_get_args();
Index: trunk/extensions/Interwiki/Interwiki.i18n.php
——@@ -51,6 +51,7 @@
5252 'interwiki_1' => 'yes',
5353 'interwiki_0' => 'no',
5454 'interwiki_error' => 'Error: The interwiki table is empty, or something else went wrong.',
 55+ 'interwiki-cached' => 'The interwiki data is cached. Modifying the cache is not possible.',
5556
5657 # modifying permitted
5758 'interwiki_edit' => 'Edit',
Index: trunk/extensions/Interwiki/Interwiki.php
——@@ -7,11 +7,11 @@
88 *
99 * @file
1010 * @ingroup Extensions
11 - * @version 1.3
 11+ * @version 2.0
1212 * @author Stephanie Amanda Stevens <phroziac@gmail.com>
13 - * @author SPQRobin <robinp.1273@gmail.com>
 13+ * @author Robin Pepermans (SPQRobin) <robinp.1273@gmail.com>
1414 * @copyright Copyright © 2005-2007 Stephanie Amanda Stevens
15 - * @copyright Copyright © 2007 SPQRobin
 15+ * @copyright Copyright © 2007-2011 Robin Pepermans (SPQRobin)
1616 * @license http://www.gnu.org/copyleft/gpl.html GNU General Public License 2.0 or later
1717 * @link http://www.mediawiki.org/wiki/Extension:SpecialInterwiki Documentation
1818 * Formatting improvements Stephen Kennedy, 2006.
——@@ -26,11 +26,17 @@
2727 'path' => __FILE__,
2828 'name' => 'SpecialInterwiki',
2929 'author' => array( 'Stephanie Amanda Stevens', 'SPQRobin', '...' ),
30 - 'version' => '1.4.1',
 30+ 'version' => '2.0',
3131 'url' => 'http://www.mediawiki.org/wiki/Extension:SpecialInterwiki',
3232 'descriptionmsg' => 'interwiki-desc',
3333 );
3434
 35+$wgResourceModules['SpecialInterwiki'] = array(
 36+ 'styles' => 'Interwiki.css',
 37+ 'localBasePath' => dirname( __FILE__ ),
 38+ 'remoteExtPath' => 'Interwiki',
 39+);
 40+
3541 // Set up the new special page
3642 $dir = dirname( __FILE__ ) . '/';
3743 $wgExtensionMessagesFiles['Interwiki'] = $dir . 'Interwiki.i18n.php';
Index: trunk/extensions/Interwiki/Interwiki.css
——@@ -10,11 +10,11 @@
1111 vertical-align: top;
1212 }
1313
14 -table.mw-interwikitable.intro th.mw-align-left {
 14+table.mw-interwikitable.intro th.mw-align-start {
1515 text-align: left;
1616 }
1717
18 -table.mw-interwikitable.intro th.mw-align-right {
 18+table.mw-interwikitable.intro th.mw-align-end {
1919 text-align: right;
2020 }
2121
——@@ -23,10 +23,13 @@
2424 table.mw-interwikitable.body td.mw-interwikitable-modify {
2525 text-align: center;
2626 }
27 -.mw-interwikitable-url {
 27+/* @noflip */ .mw-interwikitable-url {
2828 text-align: left;
2929 }
30 -input#mw-interwiki-url {
 30+/* @noflip */ input#mw-interwiki-url {
3131 text-align: left;
3232 direction: ltr;
 33+}
 34+.mw-interwikitable-modify {
 35+ white-space: nowrap;
3336 }
\ No newline at end of file

Follow-up revisions

Rev.Commit summaryAuthorDate
r92636Use wfMessage() functions, and rewrite/clean-up showForm() using Html and Xml...robin15:29, 20 July 2011
r105761fix copy-paste error in r92529 (oops)robin12:54, 10 December 2011

Past revisions this follows-up on

Rev.Commit summaryAuthorDate
r92528(bug 19838) API does not use interwiki cache....robin12:30, 19 July 2011

Comments

#Comment by Nikerabbit (talk | contribs)   12:43, 20 July 2011

Double escaping: 

+Html::element( 'th', null, wfMsgHtml( 'interwiki_local' ) ) .

In canModify(), the newest fashion is to throw Exceptions.

#Comment by SPQRobin (talk | contribs)   15:31, 20 July 2011

Done in r92636 + rewritten to use wfMessage(). I hope escaping is done right :)

Status & tagging log

  • 07:50, 21 September 2011 Nikerabbit (talk | contribs) changed the status of r92529 [removed: new added: resolved]
Retrieved from "http://www.mediawiki.org/wiki/Special:Code/MediaWiki/92529"