r89898 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r89897‎ \| r89898 \| r89899 >
Date:	22:14, 11 June 2011
Author:	robin
Status:	deferred (Comments)
Tags:
Comment:	Show an error when prefix or URL is empty, instead of adding it. Various other small fixes, including: * Show "return to Special:Interwiki" always when on the submit form * Use nice url Special:Interwiki/add instead of ...&action=add * Move inline css to css file
Modified paths:	/trunk/extensions/Interwiki/Interwiki.css (modified) (history) /trunk/extensions/Interwiki/Interwiki.i18n.php (modified) (history) /trunk/extensions/Interwiki/Interwiki.php (modified) (history) /trunk/extensions/Interwiki/Interwiki_body.php (modified) (history)

Diff [purge]

Index: trunk/extensions/Interwiki/Interwiki_body.php
—	—	@@ -38,40 +38,37 @@
39	39
40	40	$admin = $wgUser->isAllowed( 'interwiki' );
41	41	$action = $wgRequest->getVal( 'action', $par );
	42	+ $return = SpecialPage::getTitleFor( 'Interwiki' );
42	43
43	44	switch( $action ) {
44	45	case 'delete':
45	46	case 'edit':
46	47	case 'add':
47		~~- // Check permissions~~
48	48	if( !$admin ) {
	49	+ // Check permissions
49	50	$wgOut->permissionRequired( 'interwiki' );
50		~~- return;~~
51		~~- }~~
52		~~- // Is the database in read-only mode?~~
53		~~- if( wfReadOnly() ) {~~
	51	+ } elseif( wfReadOnly() ) {
	52	+ // Is the database in read-only mode?
54	53	$wgOut->readOnlyPage();
55		~~- return;~~
	54	+ } else {
	55	+ $this->showForm( $action );
56	56	}
57		~~- $this->showForm( $action );~~
	57	+ $wgOut->returnToMain( false, $return );
58	58	break;
59	59	case 'submit':
60		~~- // Check permissions~~
61	60	if( !$admin ) {
	61	+ // Check permissions
62	62	$wgOut->permissionRequired( 'interwiki' );
63		~~- return;~~
64		~~- }~~
65		~~- // Is the database in read-only mode?~~
66		~~- if( wfReadOnly() ) {~~
	63	+ } elseif( wfReadOnly() ) {
	64	+ // Is the database in read-only mode?
67	65	$wgOut->readOnlyPage();
68		~~- return;~~
69		~~- }~~
70		~~- // Prevent cross-site request forgeries~~
71		~~- if( !$wgRequest->wasPosted() \|\| !$wgUser->matchEditToken( $wgRequest->getVal( 'wpEditToken' ) ) ) {~~
	66	+ } elseif( !$wgRequest->wasPosted() \|\| !$wgUser->matchEditToken( $wgRequest->getVal( 'wpEditToken' ) ) ) {
	67	+ // Prevent cross-site request forgeries
72	68	$wgOut->addWikiMsg( 'sessionfailure' );
73		~~- return;~~
	69	+ } else {
	70	+ $this->doSubmit();
74	71	}
75		~~- $this->doSubmit();~~
	72	+ $wgOut->returnToMain( false, $return );
76	73	break;
77	74	default:
78	75	$this->showList( $admin );
—	—	@@ -99,15 +96,15 @@
100	97	Xml::element( 'legend', null, $topmessage ) .
101	98	Xml::openElement( 'form', array( 'id' => 'mw-interwiki-deleteform', 'method' => 'post', 'action' => $actionUrl ) ) .
102	99	Xml::openElement( 'table' ) .
103		~~- "<tr><td>$deletingmessage</td></tr>".~~
	100	+ "<tr><td colspan='2'>$deletingmessage</td></tr>".
104	101	'<tr><td class="mw-label">' . Xml::label( $reasonmessage, 'mw-interwiki-deletereason' ) . '</td>' .
105	102	'<td class="mw-input">' .
106	103	Xml::input( 'wpInterwikiReason', 60, '', array( 'tabindex' => '1', 'id' => 'mw-interwiki-deletereason', 'maxlength' => '200' ) ) .
107	104	'</td></tr>' .
108	105	'<tr><td></td><td class="mw-submit">' . Xml::submitButton( $button, array( 'id' => 'mw-interwiki-submit' ) ) .
109		~~- Html::Hidden( 'wpInterwikiPrefix', $prefix ) .~~
110		~~- Html::Hidden( 'wpInterwikiAction', $action ) .~~
111		~~- Html::Hidden( 'wpEditToken', $token ) .~~
	106	+ Html::hidden( 'wpInterwikiPrefix', $prefix ) .
	107	+ Html::hidden( 'wpInterwikiAction', $action ) .
	108	+ Html::hidden( 'wpEditToken', $token ) .
112	109	'</td></tr>' .
113	110	Xml::closeElement( 'table' ) .
114	111	Xml::closeElement( 'form' ) .
—	—	@@ -128,7 +125,7 @@
129	126	$defaulturl = $row->iw_url;
130	127	$trans = $row->iw_trans;
131	128	$local = $row->iw_local;
132		~~- $old = Html::Hidden( 'wpInterwikiPrefix', $row->iw_prefix );~~
	129	+ $old = Html::hidden( 'wpInterwikiPrefix', $row->iw_prefix );
133	130	$topmessage = wfMsgExt( 'interwiki_edittext', array( 'parseinline' ) );
134	131	$intromessage = wfMsgExt( 'interwiki_editintro', array( 'parseinline' ) );
135	132	$button = wfMsg( 'edit' );
—	—	@@ -138,7 +135,7 @@
139	136	$local = $wgRequest->getCheck( 'wpInterwikiLocal' );
140	137	$trans = $wgRequest->getCheck( 'wpInterwikiTrans' );
141	138	$old = '';
142		~~- $defaulturl = $wgRequest->getVal( 'wpInterwikiURL' ) ? $wgRequest->getVal( 'wpInterwikiURL' ) : wfMsg( 'interwiki_defaulturl' );~~
	139	+ $defaulturl = $wgRequest->getVal( 'wpInterwikiURL' ) ? $wgRequest->getVal( 'wpInterwikiURL' ) : wfMsg( 'interwiki-defaulturl' );
143	140	$topmessage = wfMsgExt( 'interwiki_addtext', array( 'parseinline' ) );
144	141	$intromessage = wfMsgExt( 'interwiki_addintro', array( 'parseinline' ) );
145	142	$button = wfMsg( 'interwiki_addbutton' );
—	—	@@ -164,9 +161,9 @@
165	162	'<td class="mw-input">' . Xml::input( 'wpInterwikiURL', 60, $defaulturl, array( 'tabindex' => '1', 'maxlength' => '200', 'id' => 'mw-interwiki-url' ) ) . '</td></tr>' .
166	163	'<tr><td class="mw-label">' . Xml::label( $reasonmessage, 'mw-interwiki-editreason' ) . '</td>' .
167	164	'<td class="mw-input">' . Xml::input( 'wpInterwikiReason', 60, '', array( 'tabindex' => '1', 'id' => 'mw-interwiki-editreason', 'maxlength' => '200' ) ) .
168		~~- Html::Hidden( 'wpInterwikiAction', $action ) .~~
	165	+ Html::hidden( 'wpInterwikiAction', $action ) .
169	166	$old .
170		~~- Html::Hidden( 'wpEditToken', $token ) .~~
	167	+ Html::hidden( 'wpEditToken', $token ) .
171	168	'</td></tr>' .
172	169	'<tr><td></td><td class="mw-submit">' . Xml::submitButton( $button, array( 'id' => 'mw-interwiki-submit' ) ) . '</td></tr>' .
173	170	Xml::closeElement( 'table' ) .
—	—	@@ -199,7 +196,6 @@
200	197	$this->showForm( $do );
201	198	} else {
202	199	$wgOut->addWikiMsg( 'interwiki_deleted', $prefix );
203		~~- $wgOut->returnToMain( false, $selfTitle );~~
204	200	$log = new LogPage( 'interwiki' );
205	201	$log->addEntry( 'iw_delete', $selfTitle, $reason, array( $prefix ) );
206	202	}
—	—	@@ -215,6 +211,12 @@
216	212	'iw_local' => $local,
217	213	'iw_trans' => $trans
218	214	);
	215	+
	216	+ if( $prefix == '' \|\| $theurl == '' ) {
	217	+ $this->error( 'interwiki-submit-empty' );
	218	+ $this->showForm( $do );
	219	+ return;
	220	+ }
219	221
220	222	if( $do == 'add' ){
221	223	$dbw->insert( 'interwiki', $data, __METHOD__, 'IGNORE' );
—	—	@@ -227,7 +229,6 @@
228	230	$this->showForm( $do );
229	231	} else {
230	232	$wgOut->addWikiMsg( "interwiki_{$do}ed", $prefix );
231		~~- $wgOut->returnToMain( false, $selfTitle );~~
232	233	$log = new LogPage( 'interwiki' );
233	234	$log->addEntry( 'iw_' . $do, $selfTitle, $reason, array( $prefix, $theurl, $trans, $local ) );
234	235	}
—	—	@@ -258,7 +259,7 @@
259	260	$message_1 = wfMsgHtml( 'interwiki_1' );
260	261
261	262	$out = '
262		~~-<table width="100%" class="mw-interwikitable wikitable intro">~~
	263	+<table class="mw-interwikitable wikitable intro">
263	264	<tr><th class="mw-align-left">' . $prefixmessage . '</th><td>' . wfMsgExt( 'interwiki_prefix_intro', 'parseinline' ) . '</td></tr>
264	265	<tr><th class="mw-align-left">' . $urlmessage . '</th><td>' . wfMsgExt( 'interwiki_url_intro', 'parseinline' ) . '</td></tr>
265	266	<tr><th class="mw-align-left">' . $localmessage . '</th><td>' . wfMsgExt( 'interwiki_local_intro', 'parseinline' ) . '</td></tr>
—	—	@@ -272,14 +273,13 @@
273	274	$wgOut->addWikiMsg( 'interwiki_intro' );
274	275	$wgOut->addHTML( $out );
275	276	$wgOut->addWikiMsg( 'interwiki_intro_footer' );
276		~~- $selfTitle = $this->getTitle();~~
277	277
278	278	// Privileged users can add new prefixes
279	279	if ( $admin ) {
280	280	$skin = $wgUser->getSkin();
281	281	$addtext = wfMsgHtml( 'interwiki_addtext' );
282		~~- $addlink = $skin->link( $selfTitle, $addtext, array(), array( 'action' => 'add' ) );~~
283		~~- $wgOut->addHTML( '<p>' . $addlink . '</p>' );~~
	282	+ $addlink = $skin->link( SpecialPage::getTitleFor( 'Interwiki', 'add' ), $addtext );
	283	+ $wgOut->addHTML( '<p class="mw-interwiki-addlink">' . $addlink . '</p>' );
284	284	}
285	285
286	286	$dbr = wfGetDB( DB_SLAVE );
—	—	@@ -291,8 +291,10 @@
292	292	return;
293	293	}
294	294
	295	+ $selfTitle = $this->getTitle();
	296	+
295	297	$out = "
296		~~- <table width='100%' class='mw-interwikitable wikitable sortable body'>~~
	298	+ <table class='mw-interwikitable wikitable sortable body'>
297	299	<tr id='interwikitable-header'><th>$prefixmessage</th> <th>$urlmessage</th> <th>$localmessage</th> <th>$transmessage</th>";
298	300	// Privileged users can modify and delete existing prefixes
299	301	if( $admin ) {
Index: trunk/extensions/Interwiki/Interwiki.i18n.php
—	—	@@ -46,7 +46,7 @@
47	47	'interwiki_trans_intro' => 'If wikitext syntax <code>{<nowiki />{prefix:<i>pagename</i>}}</code> is used, then:',
48	48	'interwiki_trans_1_intro' => 'allow transclusion from the foreign wiki, if interwiki transclusions are generally permitted in this wiki,',
49	49	'interwiki_trans_0_intro' => 'do not allow it, rather look for a page in the template namespace.',
50		~~- 'interwiki_intro_footer' => 'See [http://www.mediawiki.org/wiki/Interwiki_table MediaWiki.org] for more information about the interwiki table.~~
	50	+ 'interwiki_intro_footer' => 'See [http://www.mediawiki.org/wiki/Manual:Interwiki_table MediaWiki.org] for more information about the interwiki table.
51	51	There is a [[Special:Log/interwiki\|log of changes]] to the interwiki table.',
52	52	'interwiki_1' => 'yes',
53	53	'interwiki_0' => 'no',
—	—	@@ -70,16 +70,17 @@
71	71	'interwiki_added' => 'Prefix "$1" was successfully added to the interwiki table.',
72	72	'interwiki_addfailed' => 'Prefix "$1" could not be added to the interwiki table.
73	73	Possibly it already exists in the interwiki table.',
74		~~- 'interwiki_defaulturl' => 'http://www.example.com/$1', # do not translate or duplicate this message to other languages~~
	74	+ 'interwiki-defaulturl' => 'http://www.example.com/$1', # do not translate or duplicate this message to other languages
75	75
76	76	# editing a prefix
77	77	'interwiki_edittext' => 'Editing an interwiki prefix',
78	78	'interwiki_editintro' => 'You are editing an interwiki prefix.
79	79	Remember that this can break existing links.',
80	80	'interwiki_edited' => 'Prefix "$1" was successfully modified in the interwiki table.',
81		~~- 'interwiki_editerror' => 'Prefix "$1" can not be modified in the interwiki table.~~
	81	+ 'interwiki_editerror' => 'Prefix "$1" could not be modified in the interwiki table.
82	82	Possibly it does not exist.',
83	83	'interwiki-badprefix' => 'Specified interwiki prefix "$1" contains invalid characters',
	84	+ 'interwiki-submit-empty' => 'The prefix and URL cannot be empty.',
84	85
85	86	# interwiki log
86	87	'interwiki_logpagename' => 'Interwiki table log',
Index: trunk/extensions/Interwiki/Interwiki.php
—	—	@@ -9,7 +9,7 @@
10	10	* @ingroup Extensions
11	11	* @version 1.3
12	12	* @author Stephanie Amanda Stevens <phroziac@gmail.com>
13		~~- * @author SPQRobin <robin_1273@hotmail.com>~~
	13	+ * @author SPQRobin <robinp.1273@gmail.com>
14	14	* @copyright Copyright © 2005-2007 Stephanie Amanda Stevens
15	15	* @copyright Copyright © 2007 SPQRobin
16	16	* @license http://www.gnu.org/copyleft/gpl.html GNU General Public License 2.0 or later
—	—	@@ -26,7 +26,7 @@
27	27	'path' => __FILE__,
28	28	'name' => 'SpecialInterwiki',
29	29	'author' => array( 'Stephanie Amanda Stevens', 'SPQRobin', '...' ),
30		~~- 'version' => '1.3.2',~~
	30	+ 'version' => '1.4.0',
31	31	'url' => 'http://www.mediawiki.org/wiki/Extension:SpecialInterwiki',
32	32	'descriptionmsg' => 'interwiki-desc',
33	33	);
Index: trunk/extensions/Interwiki/Interwiki.css
—	—	@@ -1,6 +1,10 @@
2	2	/**
3	3	* CSS for Special:Interwiki
4	4	*/
	5	+table.mw-interwikitable {
	6	+ width: 100%;
	7	+}
	8	+
5	9	table.mw-interwikitable.intro th {
6	10	padding-right: 1.4ex;
7	11	vertical-align: top;

Sign-offs

User	Flag	Date
Nikerabbit	inspected	13:44, 20 July 2011

Follow-up revisions

Revision	Commit summary	Author	Date
r89924	Use $this->getTitle() per comment on r89898.	robin	11:45, 12 June 2011
r89937	Update per r89898	robin	20:57, 12 June 2011

Comments

#Comment by Nikerabbit (talk | contribs) 07:12, 12 June 2011

- SpecialPage::getTitleFor( 'Interwiki', 'add' )
+ $this->getTitle( 'add' );

Same goes for

+ $return = SpecialPage::getTitleFor( 'Interwiki' );

#Comment by SPQRobin (talk | contribs) 11:46, 12 June 2011

Ah, ok. Done in r89924.

Status & tagging log

00:27, 17 June 2011 Reedy (talk | contribs) changed the status of r89898 [removed: new added: deferred]