r79851 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r79850‎ | r79851 | r79852 >
Date:23:52, 7 January 2011
Author:brion
Status:ok (Comments)
Tags:
Comment:
* (bug 26625) fix regression in Special:PrefixIndex from r75314; pages weren't listed if namespace was set but no prefix given
Modified paths:
  • /trunk/phase3/includes/specials/SpecialPrefixindex.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/specials/SpecialPrefixindex.php
——@@ -54,12 +54,17 @@
5555 : wfMsg( 'prefixindex' )
5656 );
5757
 58+ $showme = '';
5859 if( isset( $par ) ){
59 - $this->showPrefixChunk( $namespace, $par, $from );
 60+ $showme = $par;
6061 } elseif( $prefix != '' ){
61 - $this->showPrefixChunk( $namespace, $prefix, $from );
 62+ $showme = $prefix;
6263 } elseif( $from != '' ){
63 - $this->showPrefixChunk( $namespace, $from, $from );
 64+ // For back-compat with Special:Allpages
 65+ $showme = $from;
 66+ }
 67+ if ($showme != '' || $namespace) {
 68+ $this->showPrefixChunk( $namespace, $showme, $from );
6469 } else {
6570 $wgOut->addHTML( $this->namespacePrefixForm( $namespace, null ) );
6671 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r799171.17: MFT r78554, r79166, r79702, r79746, r79812, r79835, r79838, r79846, r79...catrope21:07, 9 January 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r75314(bug 23923) Special:Prefixindex no longer shows results if nothing was reques...btongminh15:33, 24 October 2010

Comments

#Comment by Catrope (talk | contribs)   15:38, 8 January 2011 
+		if ($showme != '' || $namespace) {

Shouldn't you use strict comparison here, given that $showme may have been set to $par, which is only checked for null (with an evil isset(), I might add) but not checked with != like the other two?

#Comment by Brion VIBBER (talk | contribs)   18:55, 8 January 2011

So, $par is either *present* (a string value) or *not present* (null). If it's *present*, its string value is the value we use for $showme.

$prefix and $from are either *empty strings* or *non-empty strings*. If $par is not present, and one of $prefix or $from is non-empty, then that's the value we use for $showme.

In the case that $par is *not present* and $prefix and $from are both *empty strings*, then $showme remains an *empty string*.

Now, our given value ($showme) is either an *empty string* or a *non-empty string*.

$namespace is either *zero* or a *non-zero integer*.

If $showme is a *non-empty string* OR $namespace is set to a *non-zero integer*, then we show a prefix view.

If $showme is an *empty string* AND $namespace is zero, then we just show a blank form.


Is there something you would recommend changing that would make a difference in behavior?

#Comment by Catrope (talk | contribs)   22:47, 8 January 2011

I'm mostly worried about what happens when $showme ends up being 0 or something and would be considered not equal to the empty string when using weak comparison. This can't possibly happen for $from and $prefix due to the way the code is written, though, and it turns out not to be a problem for $par , so it's fine. I'm sorry for the paranoia, marking OK.

#Comment by Nikerabbit (talk | contribs)   13:33, 9 January 2011

Yeah it's ok, but it is not obvious from the code without some thinking so there is room for improvement.

Status & tagging log