r85025 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r85024‎ | r85025 | r85026 >
Date:18:00, 30 March 2011
Author:happy-melon
Status:resolved (Comments)
Tags:
Comment:
(bug 23408) give admins a warning before letting them block themselves. Also tweak the logic so that loading the interface for one already-blocked user, then actually submitting a block on a different user, shows the confirmation again if the second user is already blocked.
Modified paths:
  • /trunk/phase3/includes/specials/SpecialBlock.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
——@@ -2073,6 +2073,7 @@
20742074 'badipaddress',
20752075 'blockipsuccesssub',
20762076 'blockipsuccesstext',
 2077+ 'ipb-blockingself',
20772078 'ipb-edit-dropdown',
20782079 'ipb-unblock-addr',
20792080 'ipb-unblock',
Index: trunk/phase3/includes/specials/SpecialBlock.php
——@@ -221,7 +221,7 @@
222222 $fields['DisableUTEdit']['default'] = $block->prevents( 'editownusertalk' );
223223 }
224224 $fields['Reason']['default'] = $block->mReason;
225 - $fields['AlreadyBlocked']['default'] = true;
 225+ $fields['AlreadyBlocked']['default'] = htmlspecialchars( $block->getTarget() );
226226
227227 if( $block->mExpiry == 'infinity' ) {
228228 $fields['Expiry']['default'] = 'indefinite';
——@@ -486,11 +486,23 @@
487487 $user = $target;
488488 $target = $user->getName();
489489 $userId = $user->getId();
 490+
 491+ # Give admins a heads-up before they go and block themselves. Much messier
 492+ # to do this for IPs, but it's pretty unlikely they'd ever get the 'block'
 493+ # permission anyway, although the code does allow for it
 494+ if( $target == $wgUser->getName()
 495+ && $data['AlreadyBlocked'] != htmlspecialchars( $wgUser->getName() ) )
 496+ {
 497+ return array( 'ipb-blockingself' );
 498+ }
 499+
490500 } elseif( $type == Block::TYPE_RANGE ){
491501 $userId = 0;
 502+
492503 } elseif( $type == Block::TYPE_IP ){
493504 $target = $target->getName();
494505 $userId = 0;
 506+
495507 } else {
496508 # This should have been caught in the form field validation
497509 return array( 'badipaddress' );
——@@ -556,7 +568,7 @@
557569 $status = $block->insert();
558570 if( !$status ) {
559571 # Show form unless the user is already aware of this...
560 - if( !$data['AlreadyBlocked'] ) {
 572+ if( $data['AlreadyBlocked'] != htmlspecialchars( $block->getTarget() ) ) {
561573 return array( array( 'ipb_already_blocked', $block->getTarget() ) );
562574 # Otherwise, try to update the block...
563575 } else {
Index: trunk/phase3/languages/messages/MessagesEn.php
——@@ -3060,6 +3060,7 @@
30613061 'blockipsuccesssub' => 'Block succeeded',
30623062 'blockipsuccesstext' => '[[Special:Contributions/$1|$1]] has been blocked.<br />
30633063 See [[Special:IPBlockList|IP block list]] to review blocks.',
 3064+'ipb-blockingself' => 'You are about to block yourself! Are you sure you want to do that?',
30643065 'ipb-edit-dropdown' => 'Edit block reasons',
30653066 'ipb-unblock-addr' => 'Unblock $1',
30663067 'ipb-unblock' => 'Unblock a username or IP address',

Sign-offs

UserFlagDate
Nikerabbitinspected18:20, 30 March 2011

Follow-up revisions

RevisionCommit summaryAuthorDate
r85042Follow-up r85025 - use strict comparisonhappy-melon20:05, 30 March 2011
r85166Follow-up r85025: fix the you're-trying-to-block-yourself-you-twit warning, a...happy-melon23:13, 1 April 2011
r93233* Fix double-escaping from r85025...happy-melon19:04, 26 July 2011
r94235Followup r85025, r85166, r93233: use strict comparison per CR comment, becaus...catrope10:33, 11 August 2011

Comments

#Comment by Nikerabbit (talk | contribs)   18:20, 30 March 2011 
+if( $target == $wgUser->getName()

> var_dump( "0" == "00" );
bool(true)

Although it is ignored in html, I see no reason to use two spaces after a dot.

#Comment by Catrope (talk | contribs)   10:34, 11 August 2011

Fixed in r94235. All comments are resolved now.

#Comment by Happy-melon (talk | contribs)   20:08, 30 March 2011

:P

#Comment by Bawolff (talk | contribs)   20:42, 31 March 2011

I can't figure out how to block my self now. I get the warning, but don't see any way to say "yes as a matter of fact I really do want to block myself"

#Comment by Happy-melon (talk | contribs)   20:47, 31 March 2011

It should work if you just submit the form again, in the same way as if you try and block a user who is already blocked. Does that not work for you?

#Comment by Bawolff (talk | contribs)   21:31, 31 March 2011

no, it doesn't.

#Comment by Happy-melon (talk | contribs)   23:14, 1 April 2011

Should be fixed in r85166.

#Comment by Aaron Schulz (talk | contribs)   18:51, 18 June 2011 
$fields['AlreadyBlocked']['default'] = htmlspecialchars( $block->getTarget() );

What does the htmlspecialchars do here?

#Comment by Werdna (talk | contribs)   17:51, 15 July 2011

The htmlspecialchars are unnecessary.

HTMLForm does its own HTML escaping, unless you specifically specify for it to do otherwise.

#Comment by Happy-melon (talk | contribs)   19:05, 26 July 2011

Removed in r93233.

Status & tagging log