User "Aaron Schulz" changed the status of MediaWiki.r83909.

Old Status: new
New Status: fixme

User "Aaron Schulz" also posted a comment on MediaWiki.r83909.

Full URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/83909#c18254
Commit summary:

Complete the trinity of blocking frontend interfaces by rewriting 
SpecialIpblocklist:
* Move and rename to SpecialBlockList
* Use an HTMLForm in GET mode for the options form
* Use TablePager to organise the results more nicely
* Standardise the filtration for IPs and IP ranges, so looking at blocks for a 
range will now also show rangeblocks which contain the range
* General tidy up

Comment:

<pre>+ $formatted = wfMessage( 'autoblockid', $row->ipb_id );</pre>

How does that work? Should it return HTML?

<pre>+ static $headers = null;</pre>

Why does getFieldNames() use a static var? Is it called often or something?

<pre>line 123   if ( !$wgUser->isAllowed( 'hideuser' ) ){</pre>

This check is already there in the Pager class.

<pre>
+       protected static function getIpFragment( $ip ){
+               global $wgBlockCIDRLimit;
+               if( IP::isIPv4( $ip ) ){
+                       $hexAddress = IP::toHex( $ip );
+                       return substr( $hexAddress, 0,  wfBaseconvert( 
$wgBlockCIDRLimit['IPv4'], 10, 16 ) );
+               } elseif( IP::isIPv6( $ip ) ) {
+                       $hexAddress = substr( IP::toHex( $ip ), 2 );
+                       return 'v6-' . substr( $hexAddress, 0,  wfBaseconvert( 
$wgBlockCIDRLimit['IPv6'], 10, 16 ) );
</pre>

I don't think this works like you think. Try:
<pre>
$hexAddress = IP::toHex( '122.114.14.5' );
var_dump( $hexAddress  );
var_dump( wfBaseconvert( 16, 10, 16 ) );
var_dump( $hexAddress, 0, wfBaseconvert( 16, 10, 16 ) );
</pre>

Which gives...
<pre>
string(8) "7A720E05"
string(2) "10"
string(8) "7A720E05"
</pre>

If the min prefix is 16 bits, and each hex char is 4 bits, you'd want 
floor(16/4) = 4. Maybe use ($wgBlockCIDRLimit['IPv4']/4) in place of " 
wfBaseconvert( $wgBlockCIDRLimit['IPv4'], 10, 16 )" and likewise for v6?

_______________________________________________
MediaWiki-CodeReview mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview

Reply via email to