r83202 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r83201‎ | r83202 | r83203 >
Date:01:48, 4 March 2011
Author:krinkle
Status:ok (Comments)
Tags:
Comment:
Moving IP-check to mw.util & some enhancements to and applying code conventions to mw.special.block.js
* This commit addresses the @TODOs that were in mediawiki.legacy.block before it was migrated to /resources in r83183 ("@TODO: find some better JS file for [is IP functions]")
* JSHint warning fixed: Using ==== when comparing to '' (safer and faster than ==)
* Instead of checking the initial onload state by faking a onkeyup/onchange event, seperate the two functions and use those
** --> This fixes issue where it falsely triggers other bindings on-load that are globally bound to input elements via $('input').live() or $('[type="text"]').delegate() because of the .keyup()/.change() call )
* Caching selectors (var $wpBlockExpiry = $(..) etc.) instead of re-creating a dozen jQuery objects again and getting the document element by id – on *every* onkeyup/onchange
** --> Notable speed improvement in Safari 3 and IE6 with the caching, these had a little bit of a lag when typing fast, mw.legacy.block.js had the lag as well. Fixed now.
* Adding instantToggle-argument to switch between fade or hiding/showing right away. When setting the initial state they're called with instant=true so that there's no fading, the page loads and stuff hides that shouldn't be visible. Any later interaction will still have the fade.
** Calling a function like foo(true) sucks, adding a local DO_INSTANT = true.
* Making behaviour the same as the legacy.block was (if isEmpty, do bring the element back to view ( show() / fadeIn) instead of doing nothing)
Modified paths:
  • /trunk/phase3/resources/Resources.php (modified) (history)
  • /trunk/phase3/resources/mediawiki.special/mediawiki.special.block.js (modified) (history)
  • /trunk/phase3/resources/mediawiki.util/mediawiki.util.js (modified) (history)

Diff [purge]

Index: trunk/phase3/resources/mediawiki.util/mediawiki.util.js
——@@ -544,6 +544,32 @@
545545 'i'
546546 );
547547 return (null !== mailtxt.match( HTML5_email_regexp ) );
 548+ },
 549+ // Note: borrows from IP.php
 550+ 'isIPv4Address' : function( address, allowBlock ) {
 551+ var block = allowBlock ? '(?:\\/(?:3[0-2]|[12]?\\d))?' : '';
 552+ var RE_IP_BYTE = '(?:25[0-5]|2[0-4][0-9]|1[0-9][0-9]|0?[0-9]?[0-9])';
 553+ var RE_IP_ADD = '(?:' + RE_IP_BYTE + '\\.){3}' + RE_IP_BYTE;
 554+ return address.search( new RegExp( '^' + RE_IP_ADD + block + '$' ) ) != -1;
 555+ },
 556+ // Note: borrows from IP.php
 557+ 'isIPv6Address' : function( address, allowBlock ) {
 558+ var block = allowBlock ? '(?:\\/(?:12[0-8]|1[01][0-9]|[1-9]?\\d))?' : '';
 559+ var RE_IPV6_ADD =
 560+ '(?:' + // starts with "::" (including "::")
 561+ ':(?::|(?::' + '[0-9A-Fa-f]{1,4}' + '){1,7})' +
 562+ '|' + // ends with "::" (except "::")
 563+ '[0-9A-Fa-f]{1,4}' + '(?::' + '[0-9A-Fa-f]{1,4}' + '){0,6}::' +
 564+ '|' + // contains no "::"
 565+ '[0-9A-Fa-f]{1,4}' + '(?::' + '[0-9A-Fa-f]{1,4}' + '){7}' +
 566+ ')';
 567+ if ( address.search( new RegExp( '^' + RE_IPV6_ADD + block + '$' ) ) != -1 ) {
 568+ return true;
 569+ }
 570+ RE_IPV6_ADD = // contains one "::" in the middle (single '::' check below)
 571+ '[0-9A-Fa-f]{1,4}' + '(?:::?' + '[0-9A-Fa-f]{1,4}' + '){1,6}';
 572+ return address.search( new RegExp( '^' + RE_IPV6_ADD + block + '$' ) ) != -1
 573+ && address.search( /::/ ) != -1 && address.search( /::.*::/ ) == -1;
548574 }
549575
550576 };
Index: trunk/phase3/resources/Resources.php
——@@ -426,7 +426,6 @@
427427 ),
428428 'mediawiki.special.block' => array(
429429 'scripts' => 'resources/mediawiki.special/mediawiki.special.block.js',
430 - 'dependencies' => array( 'jquery.effects.blind' ),
431430 ),
432431 'mediawiki.special.upload' => array(
433432 // @TODO: merge in remainder of mediawiki.legacy.upload
Index: trunk/phase3/resources/mediawiki.special/mediawiki.special.block.js
——@@ -1,57 +1,68 @@
22 /* JavaScript for Special:Block */
3 -jQuery( function( $ ) {
43
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 - };
 4+// Fade or snap depending on argument
 5+jQuery.fn.goIn = function( instantToggle ) {
 6+ if ( typeof instantToggle != 'undefined' && instantToggle === true ) {
 7+ return jQuery(this).show();
 8+ }
 9+ return jQuery(this).stop( true, true ).fadeIn();
 10+};
 11+jQuery.fn.goOut = function( instantToggle ) {
 12+ if ( typeof instantToggle != 'undefined' && instantToggle === true ) {
 13+ return jQuery(this).hide();
 14+ }
 15+ return jQuery(this).stop( true, true ).fadeOut();
 16+};
2817
29 - var input = $('#mw-bi-target').val();
 18+jQuery( function( $ ) {
3019
31 - var isEmpty = ( input == "" );
32 - var isIp = isIPv4Address( input ) || isIPv6Address( input );
33 - var isIpRange = isIp && input.match(/\/\d+$/);
 20+ var DO_INSTANT = true,
 21+ $blockExpiry = $( '#wpBlockExpiry' ), $blockOther = $( '#wpBlockOther' ),
 22+ $blockTarget = $( '#mw-bi-target' ), $anonOnlyRow = $( '#wpAnonOnlyRow' ),
 23+ $enableAutoblockRow = $( '#wpEnableAutoblockRow' ),
 24+ $hideUser = $( '#wpEnableHideUser' ), $watchUser = $( '#wpEnableWatchUser' );
3425
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 - }
 26+ var considerChangingExpiryFocus = function( instant ) {
 27+ if ( $blockExpiry.val() == 'other' ) {
 28+ $blockOther.goIn( instant );
 29+ } else {
 30+ $blockOther.goOut( instant );
4831 }
49 - }).keyup();
 32+ };
 33+ var updateBlockOptions = function( instant ) {
 34+ if ( !$blockTarget.length ) {
 35+ return;
 36+ }
5037
51 - $('#wpBlockExpiry').change( function(){
52 - if( $(this).val() == 'other' ){
53 - $('#wpBlockOther').stop( true, true ).fadeIn();
 38+ var blocktarget = $.trim( $blockTarget.val() );
 39+ var isEmpty = ( blocktarget === '' );
 40+ var isIp = mw.util.isIPv4Address( blocktarget, true ) || mw.util.isIPv6Address( blocktarget, true );
 41+ var isIpRange = isIp && blocktarget.match( /\/\d+$/ );
 42+
 43+ if ( isIp && !isEmpty ) {
 44+ $enableAutoblockRow.goOut( instant );
 45+ $hideUser.goOut( instant );
5446 } else {
55 - $('#wpBlockOther').stop( true, true ).fadeOut();
 47+ $enableAutoblockRow.goIn( instant );
 48+ $hideUser.goIn( instant );
5649 }
57 - }).change();
58 -} );
\ No newline at end of file
 50+ if ( !isIp && !isEmpty ) {
 51+ $anonOnlyRow.goOut( instant );
 52+ } else {
 53+ $anonOnlyRow.goIn( instant );
 54+ }
 55+ if ( isIpRange && !isEmpty ) {
 56+ $watchUser.goOut( instant );
 57+ } else {
 58+ $watchUser.goIn( instant );
 59+ }
 60+ };
 61+
 62+ // Bind functions so they're checked whenever stuff changes
 63+ $blockExpiry.change( considerChangingExpiryFocus );
 64+ $blockTarget.keyup( updateBlockOptions );
 65+
 66+ // Call them now to set initial state (ie. Special:Block/Foobar?wpBlockExpiry=2+hours)
 67+ considerChangingExpiryFocus( DO_INSTANT );
 68+ updateBlockOptions( DO_INSTANT );
 69+});
\ No newline at end of file

Follow-up revisions

RevisionCommit summaryAuthorDate
r83280Follow-up r83183, r83202:...happy-melon12:48, 5 March 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r83183Convert Special:BlockIP JS to jQuery/ResourceLoader. Change the order of the...happy-melon23:54, 3 March 2011

Comments

#Comment by Aaron Schulz (talk | contribs)   03:18, 4 March 2011

CheckUser uses the IP stuff.

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

If the functions were used outside the Special:Block page and/or if mediawiki.legacy.block was used as a resource by extensions (ie. wgOut->addModules('mediawiki.legacy.block'), then it broke in r83183, not this one. That revision changed it from window./global availability to local scope (which woudl brake CU). In this revision it changed from local scope back to the global mw.util object.

See comments on r83183.

#Comment by Aaron Schulz (talk | contribs)   20:18, 4 March 2011

Right, the follow up is to change the CU dep to mw.util. I just put it here for convenience.

Status & tagging log