r83183 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r83182‎ | r83183 | r83184 >
Date:23:54, 3 March 2011
Author:happy-melon
Status:ok (Comments)
Tags:
Comment:
Convert Special:BlockIP JS to jQuery/ResourceLoader. Change the order of the checkboxes on the block form so that options which appear/disappear when the type of user is changed are grouped together.
Modified paths:
  • /trunk/phase3/includes/specials/SpecialBlockip.php (modified) (history)
  • /trunk/phase3/resources/Resources.php (modified) (history)
  • /trunk/phase3/resources/mediawiki.special/mediawiki.special.block.js (added) (history)
  • /trunk/phase3/skins/common/block.js (deleted) (history)

Diff [purge]

Index: trunk/phase3/skins/common/block.js
——@@ -1,89 +0,0 @@
2 -// @TODO: find some better JS file for this
3 -// Note: borrows from IP.php
4 -window.isIPv4Address = function( address, allowBlock ) {
5 - var block = allowBlock ? '(?:\\/(?:3[0-2]|[12]?\\d))?' : '';
6 - var RE_IP_BYTE = '(?:25[0-5]|2[0-4][0-9]|1[0-9][0-9]|0?[0-9]?[0-9])';
7 - var RE_IP_ADD = '(?:' + RE_IP_BYTE + '\\.){3}' + RE_IP_BYTE;
8 - return address.search( new RegExp( '^' + RE_IP_ADD + block + '$' ) ) != -1;
9 -};
10 -
11 -// @TODO: find some better JS file for this
12 -// Note: borrows from IP.php
13 -window.isIPv6Address = function( address, allowBlock ) {
14 - var block = allowBlock ? '(?:\\/(?:12[0-8]|1[01][0-9]|[1-9]?\\d))?' : '';
15 - var RE_IPV6_ADD =
16 - '(?:' + // starts with "::" (including "::")
17 - ':(?::|(?::' + '[0-9A-Fa-f]{1,4}' + '){1,7})' +
18 - '|' + // ends with "::" (except "::")
19 - '[0-9A-Fa-f]{1,4}' + '(?::' + '[0-9A-Fa-f]{1,4}' + '){0,6}::' +
20 - '|' + // contains no "::"
21 - '[0-9A-Fa-f]{1,4}' + '(?::' + '[0-9A-Fa-f]{1,4}' + '){7}' +
22 - ')';
23 - if ( address.search( new RegExp( '^' + RE_IPV6_ADD + block + '$' ) ) != -1 ) {
24 - return true;
25 - }
26 - var RE_IPV6_ADD = // contains one "::" in the middle (single '::' check below)
27 - '[0-9A-Fa-f]{1,4}' + '(?:::?' + '[0-9A-Fa-f]{1,4}' + '){1,6}';
28 - return address.search( new RegExp( '^' + RE_IPV6_ADD + block + '$' ) ) != -1
29 - && address.search( /::/ ) != -1 && address.search( /::.*::/ ) == -1;
30 -};
31 -
32 -window.considerChangingExpiryFocus = function() {
33 - if ( !document.getElementById ) {
34 - return;
35 - }
36 - var drop = document.getElementById( 'wpBlockExpiry' );
37 - if ( !drop ) {
38 - return;
39 - }
40 - var field = document.getElementById( 'wpBlockOther' );
41 - if ( !field ) {
42 - return;
43 - }
44 - var opt = drop.value;
45 - if ( opt == 'other' ) {
46 - field.style.display = '';
47 - } else {
48 - field.style.display = 'none';
49 - }
50 -};
51 -
52 -window.updateBlockOptions = function() {
53 - if ( !document.getElementById ) {
54 - return;
55 - }
56 -
57 - var target = document.getElementById( 'mw-bi-target' );
58 - if ( !target ) {
59 - return;
60 - }
61 -
62 - var addy = target.value.replace( /(^\s*|\s*$)/, '' ); // trim
63 - var isEmpty = (addy == "");
64 -
65 - var isIp = isIPv4Address( addy, true ) || isIPv6Address( addy, true );
66 - var isIpRange = isIp && addy.match(/\/\d+$/);
67 -
68 - var anonymousRow = document.getElementById( 'wpAnonOnlyRow' );
69 - if( anonymousRow ) {
70 - anonymousRow.style.display = ( !isIp && !isEmpty ) ? 'none' : '';
71 - }
72 -
73 - var autoblockRow = document.getElementById( 'wpEnableAutoblockRow' );
74 - if( autoblockRow ) {
75 - autoblockRow.style.display = isIp && !isEmpty ? 'none' : '';
76 - }
77 -
78 - var hideuserRow = document.getElementById( 'wpEnableHideUser' );
79 - if( hideuserRow ) {
80 - hideuserRow.style.display = isIp && !isEmpty ? 'none' : '';
81 - }
82 -
83 - var watchuserRow = document.getElementById( 'wpEnableWatchUser' );
84 - if( watchuserRow ) {
85 - watchuserRow.style.display = isIpRange && !isEmpty ? 'none' : '';
86 - }
87 -};
88 -
89 -addOnloadHook( updateBlockOptions );
90 -addOnloadHook( considerChangingExpiryFocus );
Index: trunk/phase3/includes/specials/SpecialBlockip.php
——@@ -199,7 +199,7 @@
200200 wfMsgForContent( 'ipbreason-dropdown' ),
201201 wfMsgForContent( 'ipbreasonotherlist' ), $this->BlockReasonList, 'wpBlockDropDown', 4 );
202202
203 - $wgOut->addModules( 'mediawiki.legacy.block' );
 203+ $wgOut->addModules( 'mediawiki.special.block' );
204204 $wgOut->addHTML(
205205 Xml::openElement( 'form', array( 'method' => 'post', 'action' => $titleObj->getLocalURL( 'action=submit' ), 'id' => 'blockip' ) ) .
206206 Xml::openElement( 'fieldset' ) .
——@@ -269,14 +269,6 @@
270270 ) + ( $this->BlockAddress ? array( 'autofocus' ) : array() ) ) . "
271271 </td>
272272 </tr>
273 - <tr id='wpAnonOnlyRow'>
274 - <td>&#160;</td>
275 - <td class='mw-input'>" .
276 - Xml::checkLabel( wfMsg( 'ipbanononly' ),
277 - 'wpAnonOnly', 'wpAnonOnly', $this->BlockAnonOnly,
278 - array( 'tabindex' => '6' ) ) . "
279 - </td>
280 - </tr>
281273 <tr id='wpCreateAccountRow'>
282274 <td>&#160;</td>
283275 <td class='mw-input'>" .
——@@ -284,14 +276,6 @@
285277 'wpCreateAccount', 'wpCreateAccount', $this->BlockCreateAccount,
286278 array( 'tabindex' => '7' ) ) . "
287279 </td>
288 - </tr>
289 - <tr id='wpEnableAutoblockRow'>
290 - <td>&#160;</td>
291 - <td class='mw-input'>" .
292 - Xml::checkLabel( wfMsg( 'ipbenableautoblock' ),
293 - 'wpEnableAutoblock', 'wpEnableAutoblock', $this->BlockEnableAutoblock,
294 - array( 'tabindex' => '8' ) ) . "
295 - </td>
296280 </tr>"
297281 );
298282
——@@ -308,6 +292,32 @@
309293 );
310294 }
311295
 296+ # Can we explicitly disallow the use of user_talk?
 297+ global $wgBlockAllowsUTEdit;
 298+ if( $wgBlockAllowsUTEdit ){
 299+ $wgOut->addHTML("
 300+ <tr id='wpAllowUsertalkRow'>
 301+ <td>&#160;</td>
 302+ <td class='mw-input'>" .
 303+ Xml::checkLabel( wfMsg( 'ipballowusertalk' ),
 304+ 'wpAllowUsertalk', 'wpAllowUsertalk', $this->BlockAllowUsertalk,
 305+ array( 'tabindex' => '12' ) ) . "
 306+ </td>
 307+ </tr>"
 308+ );
 309+ }
 310+
 311+ $wgOut->addHTML( "
 312+ <tr id='wpEnableAutoblockRow'>
 313+ <td>&#160;</td>
 314+ <td class='mw-input'>" .
 315+ Xml::checkLabel( wfMsg( 'ipbenableautoblock' ),
 316+ 'wpEnableAutoblock', 'wpEnableAutoblock', $this->BlockEnableAutoblock,
 317+ array( 'tabindex' => '8' ) ) . "
 318+ </td>
 319+ </tr>"
 320+ );
 321+
312322 // Allow some users to hide name from block log, blocklist and listusers
313323 if( $wgUser->isAllowed( 'hideuser' ) ) {
314324 $wgOut->addHTML("
——@@ -337,22 +347,15 @@
338348 );
339349 }
340350
341 - # Can we explicitly disallow the use of user_talk?
342 - global $wgBlockAllowsUTEdit;
343 - if( $wgBlockAllowsUTEdit ){
344 - $wgOut->addHTML("
345 - <tr id='wpAllowUsertalkRow'>
346 - <td>&#160;</td>
347 - <td class='mw-input'>" .
348 - Xml::checkLabel( wfMsg( 'ipballowusertalk' ),
349 - 'wpAllowUsertalk', 'wpAllowUsertalk', $this->BlockAllowUsertalk,
350 - array( 'tabindex' => '12' ) ) . "
351 - </td>
352 - </tr>"
353 - );
354 - }
355 -
356351 $wgOut->addHTML("
 352+ <tr id='wpAnonOnlyRow'>
 353+ <td>&#160;</td>
 354+ <td class='mw-input'>" .
 355+ Xml::checkLabel( wfMsg( 'ipbanononly' ),
 356+ 'wpAnonOnly', 'wpAnonOnly', $this->BlockAnonOnly,
 357+ array( 'tabindex' => '6' ) ) . "
 358+ </td>
 359+ </tr>
357360 <tr>
358361 <td style='padding-top: 1em'>&#160;</td>
359362 <td class='mw-submit' style='padding-top: 1em'>" .
Index: trunk/phase3/resources/Resources.php
——@@ -424,6 +424,10 @@
425425 'mediawiki.special.search' => array(
426426 'scripts' => 'resources/mediawiki.special/mediawiki.special.search.js',
427427 ),
 428+ 'mediawiki.special.block' => array(
 429+ 'scripts' => 'resources/mediawiki.special/mediawiki.special.block.js',
 430+ 'dependencies' => array( 'jquery.effects.blind' ),
 431+ ),
428432 'mediawiki.special.upload' => array(
429433 // @TODO: merge in remainder of mediawiki.legacy.upload
430434 'scripts' => 'resources/mediawiki.special/mediawiki.special.upload.js',
Index: trunk/phase3/resources/mediawiki.special/mediawiki.special.block.js
——@@ -0,0 +1,57 @@
 2+/* JavaScript for Special:Block */
 3+jQuery( function( $ ) {
 4+
 5+ $('#mw-bi-target').keyup(function(){
 6+ var isIPv4Address = function( address ) {
 7+ var RE_IP_BYTE = '(?:25[0-5]|2[0-4][0-9]|1[0-9][0-9]|0?[0-9]?[0-9])';
 8+ var RE_IP_ADD = '(?:' + RE_IP_BYTE + '\\.){3}' + RE_IP_BYTE;
 9+ return address.search( new RegExp( '^' + RE_IP_ADD + '(?:\\/(?:3[0-2]|[12]?\\d))?$' ) ) != -1;
 10+ };
 11+ var isIPv6Address = function( address ) {
 12+ var RE_IPV6_ADD =
 13+ '(?:' + // starts with "::" (including "::")
 14+ ':(?::|(?::' + '[0-9A-Fa-f]{1,4}' + '){1,7})' +
 15+ '|' + // ends with "::" (except "::")
 16+ '[0-9A-Fa-f]{1,4}' + '(?::' + '[0-9A-Fa-f]{1,4}' + '){0,6}::' +
 17+ '|' + // contains no "::"
 18+ '[0-9A-Fa-f]{1,4}' + '(?::' + '[0-9A-Fa-f]{1,4}' + '){7}' +
 19+ ')';
 20+ if ( address.search( new RegExp( '^' + RE_IPV6_ADD + '(?:\\/(?:12[0-8]|1[01][0-9]|[1-9]?\\d))?$' ) ) != -1 ) {
 21+ return true;
 22+ }
 23+ var RE_IPV6_ADD_SHORT = // contains one "::" in the middle (single '::' check below)
 24+ '[0-9A-Fa-f]{1,4}' + '(?:::?' + '[0-9A-Fa-f]{1,4}' + '){1,6}';
 25+ return address.search( new RegExp( '^' + RE_IPV6_ADD_SHORT + '(?:\\/(?:12[0-8]|1[01][0-9]|[1-9]?\\d))?$' ) ) != -1
 26+ && address.search( /::/ ) != -1 && address.search( /::.*::/ ) == -1;
 27+ };
 28+
 29+ var input = $('#mw-bi-target').val();
 30+
 31+ var isEmpty = ( input == "" );
 32+ var isIp = isIPv4Address( input ) || isIPv6Address( input );
 33+ var isIpRange = isIp && input.match(/\/\d+$/);
 34+
 35+ if( !isEmpty ){
 36+ if( isIp ){
 37+ $( '#wpAnonOnlyRow' ).stop( true, true ).delay(1000).fadeIn();
 38+ $( '#wpEnableAutoblockRow, #wpEnableHideUser' ).stop( true, true ).delay(1000).fadeOut();
 39+ } else {
 40+ $( '#wpAnonOnlyRow' ).stop( true, true ).delay(1000).fadeOut();
 41+ $( '#wpEnableAutoblockRow, #wpEnableHideUser' ).stop( true, true ).delay(1000).fadeIn();
 42+ }
 43+ if( isIpRange ){
 44+ $( '#wpEnableWatchUser' ).stop( true, true ).delay(1000).fadeOut();
 45+ } else {
 46+ $( '#wpEnableWatchUser' ).stop( true, true ).delay(1000).fadeIn();
 47+ }
 48+ }
 49+ }).keyup();
 50+
 51+ $('#wpBlockExpiry').change( function(){
 52+ if( $(this).val() == 'other' ){
 53+ $('#wpBlockOther').stop( true, true ).fadeIn();
 54+ } else {
 55+ $('#wpBlockOther').stop( true, true ).fadeOut();
 56+ }
 57+ }).change();
 58+} );
\ No newline at end of file
Property changes on: trunk/phase3/resources/mediawiki.special/mediawiki.special.block.js
___________________________________________________________________
Added: svn:eol-style
159 + native

Sign-offs

UserFlagDate
Krinkleinspected03:16, 29 May 2011

Follow-up revisions

RevisionCommit summaryAuthorDate
r83191PHP Warning: filemtime() [function.filemtime]: stat failed for /www/w/skins/...krinkle00:20, 4 March 2011
r83194Call to two undefined functions. fixed, Follow-up r83183krinkle00:43, 4 March 2011
r83202Moving IP-check to mw.util & some enhancements to and applying code conventio...krinkle01:48, 4 March 2011
r83280Follow-up r83183, r83202:...happy-melon12:48, 5 March 2011

Comments

#Comment by Krinkle (talk | contribs)   00:02, 4 March 2011

When viewing Special:Block/Foobar I see a ticked checkbox in front of "Block anonymous users only", and, 1 second after the page has finished loading, the box and the label starts fading away. This didn't happen before this revision.

#Comment by Krinkle (talk | contribs)   00:06, 4 March 2011

Also when typing an IP-adres, say 127.0.0.1, when I type the last character again this 1 second delay and then stuff fades and moves, this is a bit counterintuitive in my opinion. A fade is shiny but the delay and the fade seem to work against user experience rather than enhance it, in this use case.

#Comment by Krinkle (talk | contribs)   00:21, 4 March 2011

PHP Warning on TranslateWiki: 

PHP Warning:  filemtime() [<a href='function.filemtime'>function.filemtime</a>]: stat failed for /www/w/skins/common/block.js in /www/w/includes/resourceloader/ResourceLoaderFileModule.php on line 365

Fixed in r83191.

#Comment by Krinkle (talk | contribs)   00:43, 4 March 2011

When typing something or changing the dropdown, these errors are thrown: 

ReferenceError: Can't find variable: updateBlockOptions
ReferenceError: Can't find variable: considerChangingExpiryFocus

Fixed in r83194.

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

Thanks Krinkle. Are there any remaining issues with this?

#Comment by Krinkle (talk | contribs)   11:23, 4 March 2011

I haven't confirmed it yet (search job is enqueued), but according to a comment r83202 (since these functions were global in window. they could be used anywhere like orignally was the case from the onchange="" attribute) the functions were used by others as well.

Guessing nobdy grep searched before making them local variables, I've ran a few searches (will take a few minutes since they each grep through the entire phase3 + extensions trunk)

#Comment by Krinkle (talk | contribs)   11:29, 4 March 2011

Not at home so can't commit right now but from what I see:

> "updateBlockOptions" in /trunk OK!

> "considerChangingExpiryFocus" in /trunk Used by GlobalBlocking extension

> "isIPv6Address" in /trunk Used by CheckUser extension

> "isIPv4Address" in /trunk Used by CheckUser extension

> "mediawiki.legacy.block" in /trunk Used by CheckUser extension

> "block.js" in /trunk OK! (although the comment about "in the spirit of" could be updated from block.js to mediawiki.special.block.js...)

#Comment by Krinkle (talk | contribs)   11:31, 4 March 2011

Second attempt:

Not at home so can't commit right now but from what I see:

  • "updateBlockOptions" in /trunk
OK!
  • "considerChangingExpiryFocus" in /trunk
Used by GlobalBlocking extension
  • "isIPv6Address" in /trunk
Used by CheckUser extension
  • "isIPv4Address" in /trunk
Used by CheckUser extension
  • "mediawiki.legacy.block" in /trunk
Used by CheckUser extension
  • "block.js" in /trunk
OK! (although the comment about "in the spirit of" could be updated from block.js to mediawiki.special.block.js...)
#Comment by Happy-melon (talk | contribs)   12:50, 5 March 2011

All fixed in r83280, I believe.

Status & tagging log