r39708 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r39707‎ | r39708 | r39709 >
Date:16:13, 20 August 2008
Author:brion
Status:old
Tags:
Comment:
Revert r39416, 39421, 39427, 39489, 39515, 39516 "Rewrite allpages to use a "tree" of index range subpages (similar to the starting page) rather than one increasingly large range list (bug 13902)"

Breaks several aspects of paging behavior:
* Subpage parameter seems to have incorrectly changed from starting point to prefix
* We've lost forward and backward paging links
Modified paths:
  • /trunk/phase3/includes/specials/SpecialAllpages.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
——@@ -1285,7 +1285,6 @@
12861286 'nextpage',
12871287 'prevpage',
12881288 'allpagesfrom',
1289 - 'allpagesto',
12901289 'allarticles',
12911290 'allinnamespace',
12921291 'allnotinnamespace',
Index: trunk/phase3/includes/specials/SpecialAllpages.php
——@@ -2,7 +2,6 @@
33 /**
44 * @file
55 * @ingroup SpecialPage
6 - * Note that Special:Prefixindex.php depends on this
76 */
87
98 /**
——@@ -14,23 +13,24 @@
1514 global $wgRequest, $wgOut, $wgContLang;
1615
1716 # GET values
18 - $from = $wgRequest->getVal( 'from', '' );
19 - $to = $wgRequest->getVal( 'to', '' );
 17+ $from = $wgRequest->getVal( 'from' );
2018 $namespace = $wgRequest->getInt( 'namespace' );
2119
2220 $namespaces = $wgContLang->getNamespaces();
2321
 22+ $indexPage = new SpecialAllpages();
 23+
2424 $wgOut->setPagetitle( ( $namespace > 0 && in_array( $namespace, array_keys( $namespaces) ) ) ?
2525 wfMsg( 'allinnamespace', str_replace( '_', ' ', $namespaces[$namespace] ) ) :
2626 wfMsg( 'allarticles' )
2727 );
28 -
29 - if( isset($par) ) {
30 - $indexPage = new SpecialPrefixIndex();
31 - $indexPage->showPrefixChunk( $namespace, $par, $specialPage->including(), $from );
 28+
 29+ if ( isset($par) ) {
 30+ $indexPage->showChunk( $namespace, $par, $specialPage->including() );
 31+ } elseif ( isset($from) ) {
 32+ $indexPage->showChunk( $namespace, $from, $specialPage->including() );
3233 } else {
33 - $indexPage = new SpecialAllpages();
34 - $indexPage->showToplevel( $namespace, $from, $to, $specialPage->including() );
 34+ $indexPage->showToplevel ( $namespace, $specialPage->including() );
3535 }
3636 }
3737
——@@ -40,19 +40,9 @@
4141 */
4242 class SpecialAllpages {
4343 /**
44 - * Maximum number of pages to show on single list subpage.
 44+ * Maximum number of pages to show on single subpage.
4545 */
46 - protected $maxPerPage = 345;
47 -
48 - /**
49 - * Maximum number of pages to show on single index subpage.
50 - */
51 - protected $maxLineCount = 200;
52 -
53 - /**
54 - * Maximum number of chars to show for an entry.
55 - */
56 - protected $maxPageLength = 70;
 46+ protected $maxPerPage = 960;
5747
5848 /**
5949 * Name of this special page. Used to make title objects that reference back
——@@ -60,13 +50,17 @@
6151 */
6252 protected $name = 'Allpages';
6353
 54+ /**
 55+ * Determines, which message describes the input field 'nsfrom'.
 56+ */
 57+ protected $nsfromMsg = 'allpagesfrom';
 58+
6459 /**
6560 * HTML for the top form
6661 * @param integer $namespace A namespace constant (default NS_MAIN).
67 - * @param string $from dbKey we are starting listing at.
68 - * @param string $to dbKey we are ending listing at.
 62+ * @param string $from Article name we are starting listing at.
6963 */
70 -function namespaceForm( $namespace = NS_MAIN, $from = '', $to = '' ) {
 64+function namespaceForm ( $namespace = NS_MAIN, $from = '' ) {
7165 global $wgScript;
7266 $t = SpecialPage::getTitleFor( $this->name );
7367
——@@ -78,22 +72,14 @@
7973 $out .= Xml::openElement( 'table', array( 'id' => 'nsselect', 'class' => 'allpages' ) );
8074 $out .= "<tr>
8175 <td class='mw-label'>" .
82 - Xml::label( wfMsg( 'allpagesfrom' ), 'nsfrom' ) .
 76+ Xml::label( wfMsg( $this->nsfromMsg ), 'nsfrom' ) .
8377 "</td>
8478 <td class='mw-input'>" .
85 - Xml::input( 'from', 30, str_replace('_',' ',$from), array( 'id' => 'nsfrom' ) ) .
 79+ Xml::input( 'from', 20, $from, array( 'id' => 'nsfrom' ) ) .
8680 "</td>
8781 </tr>
8882 <tr>
8983 <td class='mw-label'>" .
90 - Xml::label( wfMsg( 'allpagesto' ), 'nsto' ) .
91 - "</td>
92 - <td class='mw-input'>" .
93 - Xml::input( 'to', 30, str_replace('_',' ',$to), array( 'id' => 'nsto' ) ) .
94 - "</td>
95 - </tr>
96 - <tr>
97 - <td class='mw-label'>" .
9884 Xml::label( wfMsg( 'namespace' ), 'namespace' ) .
9985 "</td>
10086 <td class='mw-input'>" .
——@@ -111,7 +97,7 @@
11298 /**
11399 * @param integer $namespace (default NS_MAIN)
114100 */
115 -function showToplevel( $namespace = NS_MAIN, $from = '', $to = '', $including = false ) {
 101+function showToplevel ( $namespace = NS_MAIN, $including = false ) {
116102 global $wgOut, $wgContLang;
117103 $align = $wgContLang->isRtl() ? 'left' : 'right';
118104
——@@ -121,54 +107,45 @@
122108 $dbr = wfGetDB( DB_SLAVE );
123109 $out = "";
124110 $where = array( 'page_namespace' => $namespace );
125 -
126 - $from = Title::makeTitleSafe( $namespace, $from );
127 - $to = Title::makeTitleSafe( $namespace, $to );
128 - $from = ( $from && $from->isLocal() ) ? $from->getDBKey() : null;
129 - $to = ( $to && $to->isLocal() ) ? $to->getDBKey() : null;
130 -
131 - if( isset($from) )
132 - $where[] = 'page_title >= '.$dbr->addQuotes( $from );
133 - if( isset($to) )
134 - $where[] = 'page_title <= '.$dbr->addQuotes( $to );
135111
136112 global $wgMemc;
137 - $key = wfMemcKey( 'allpages', 'ns', $namespace, $from, $to );
 113+ $key = wfMemcKey( 'allpages', 'ns', $namespace );
138114 $lines = $wgMemc->get( $key );
139 -
140 - $count = $dbr->estimateRowCount( 'page', '*', $where, __METHOD__ );
141 - $maxPerSubpage = intval($count/$this->maxLineCount);
142 - $maxPerSubpage = max($maxPerSubpage,$this->maxPerPage);
143115
144116 if( !is_array( $lines ) ) {
145117 $options = array( 'LIMIT' => 1 );
146 - $options['ORDER BY'] = 'page_title ASC';
 118+ if ( ! $dbr->implicitOrderby() ) {
 119+ $options['ORDER BY'] = 'page_title';
 120+ }
147121 $firstTitle = $dbr->selectField( 'page', 'page_title', $where, __METHOD__, $options );
148122 $lastTitle = $firstTitle;
 123+
149124 # This array is going to hold the page_titles in order.
150125 $lines = array( $firstTitle );
 126+
151127 # If we are going to show n rows, we need n+1 queries to find the relevant titles.
152128 $done = false;
153 - while( !$done ) {
 129+ for( $i = 0; !$done; ++$i ) {
154130 // Fetch the last title of this chunk and the first of the next
155131 $chunk = is_null( $lastTitle )
156132 ? ''
157133 : 'page_title >= ' . $dbr->addQuotes( $lastTitle );
158 - $chunk = array($chunk);
159 - $res = $dbr->select( 'page', /* FROM */
 134+ $res = $dbr->select(
 135+ 'page', /* FROM */
160136 'page_title', /* WHAT */
161 - array_merge($where,$chunk),
 137+ $where + array($chunk),
162138 __METHOD__,
163 - array ('LIMIT' => 2, 'OFFSET' => $maxPerSubpage - 1, 'ORDER BY' => 'page_title ASC')
164 - );
 139+ array ('LIMIT' => 2, 'OFFSET' => $this->maxPerPage - 1, 'ORDER BY' => 'page_title') );
165140
166 - if( $s = $dbr->fetchObject( $res ) ) {
 141+ if ( $s = $dbr->fetchObject( $res ) ) {
167142 array_push( $lines, $s->page_title );
168143 } else {
169144 // Final chunk, but ended prematurely. Go back and find the end.
170145 $endTitle = $dbr->selectField( 'page', 'MAX(page_title)',
171 - array_merge($where,$chunk),
172 - __METHOD__ );
 146+ array(
 147+ 'page_namespace' => $namespace,
 148+ $chunk
 149+ ), __METHOD__ );
173150 array_push( $lines, $endTitle );
174151 $done = true;
175152 }
——@@ -188,39 +165,35 @@
189166 // If there are only two or less sections, don't even display them.
190167 // Instead, display the first section directly.
191168 if( count( $lines ) <= 2 ) {
192 - if( !empty($lines) ) {
193 - $this->showChunk( $namespace, $lines[0], $lines[count($lines)-1], $including );
194 - } else {
195 - $wgOut->addHtml( $this->namespaceForm( $namespace, $from, $to ) );
196 - }
 169+ $this->showChunk( $namespace, '', $including );
197170 return;
198171 }
199172
200173 # At this point, $lines should contain an even number of elements.
201174 $out .= "<table class='allpageslist' style='background: inherit;'>";
202 - while( count ( $lines ) > 0 ) {
203 - $inpoint = array_shift( $lines );
204 - $outpoint = array_shift( $lines );
205 - $out .= $this->showline( $inpoint, $outpoint, $namespace );
 175+ while ( count ( $lines ) > 0 ) {
 176+ $inpoint = array_shift ( $lines );
 177+ $outpoint = array_shift ( $lines );
 178+ $out .= $this->showline ( $inpoint, $outpoint, $namespace, false );
206179 }
207180 $out .= '</table>';
208 - $nsForm = $this->namespaceForm( $namespace, $from, $to );
 181+ $nsForm = $this->namespaceForm( $namespace, '', false );
209182
210183 # Is there more?
211 - if( $including ) {
 184+ if ( $including ) {
212185 $out2 = '';
213186 } else {
214 - if( isset($from) || isset($to) ) {
215 - global $wgUser;
 187+ $morelinks = '';
 188+ if ( $morelinks != '' ) {
216189 $out2 = '<table style="background: inherit;" width="100%" cellpadding="0" cellspacing="0" border="0">';
217190 $out2 .= '<tr valign="top"><td>' . $nsForm;
218 - $out2 .= '</td><td align="' . $align . '" style="font-size: smaller; margin-bottom: 1em;">' .
219 - $wgUser->getSkin()->makeKnownLink( $wgContLang->specialPage( "Allpages" ), wfMsgHtml ( 'allpages' ) );
220 - $out2 .= "</td></tr></table><hr />";
 191+ $out2 .= '</td><td align="' . $align . '" style="font-size: smaller; margin-bottom: 1em;">';
 192+ $out2 .= $morelinks . '</td></tr></table><hr />';
221193 } else {
222194 $out2 = $nsForm . '<hr />';
223195 }
224196 }
 197+
225198 $wgOut->addHtml( $out2 . $out );
226199 }
227200
——@@ -234,17 +207,14 @@
235208 $align = $wgContLang->isRtl() ? 'left' : 'right';
236209 $inpointf = htmlspecialchars( str_replace( '_', ' ', $inpoint ) );
237210 $outpointf = htmlspecialchars( str_replace( '_', ' ', $outpoint ) );
238 - // Don't let the length runaway
239 - $inpointf = $wgContLang->truncate( $inpointf, $this->maxPageLength, '...' );
240 - $outpointf = $wgContLang->truncate( $outpointf, $this->maxPageLength, '...' );
 211+ $queryparams = ($namespace ? "namespace=$namespace" : '');
 212+ $special = SpecialPage::getTitleFor( $this->name, $inpoint );
 213+ $link = $special->escapeLocalUrl( $queryparams );
241214
242 - $queryparams = $namespace ? "namespace=$namespace&" : '';
243 - $special = SpecialPage::getTitleFor( $this->name );
244 - $link = $special->escapeLocalUrl( $queryparams . 'from=' . urlencode($inpoint) . '&to=' . urlencode($outpoint) );
245 -
246 - $out = wfMsgHtml( 'alphaindexline',
247 - "<a href=\"$link\">$inpointf</a></td><td>",
248 - "</td><td><a href=\"$link\">$outpointf</a>"
 215+ $out = wfMsgHtml(
 216+ 'alphaindexline',
 217+ "<a href=\"$link\">$inpointf</a></td><td><a href=\"$link\">",
 218+ "</a></td><td><a href=\"$link\">$outpointf</a>"
249219 );
250220 return '<tr><td align="' . $align . '">'.$out.'</td></tr>';
251221 }
——@@ -252,22 +222,19 @@
253223 /**
254224 * @param integer $namespace (Default NS_MAIN)
255225 * @param string $from list all pages from this name (default FALSE)
256 - * @param string $from list all pages to this name (default FALSE)
257226 */
258 -function showChunk( $namespace = NS_MAIN, $from = false, $to = false, $including = false ) {
 227+function showChunk( $namespace = NS_MAIN, $from, $including = false ) {
259228 global $wgOut, $wgUser, $wgContLang;
260229
261230 $sk = $wgUser->getSkin();
262231
263 - $fromList = $this->getNamespaceKeyAndText( $namespace, $from );
264 - $toList = $this->getNamespaceKeyAndText( $namespace, $to );
265 -
 232+ $fromList = $this->getNamespaceKeyAndText($namespace, $from);
266233 $namespaces = $wgContLang->getNamespaces();
267234 $align = $wgContLang->isRtl() ? 'left' : 'right';
268235
269236 $n = 0;
270237
271 - if ( !$fromList || !$toList ) {
 238+ if ( !$fromList ) {
272239 $out = wfMsgWikiHtml( 'allpagesbadtitle' );
273240 } elseif ( !in_array( $namespace, array_keys( $namespaces ) ) ) {
274241 // Show errormessage and reset to NS_MAIN
——@@ -275,19 +242,17 @@
276243 $namespace = NS_MAIN;
277244 } else {
278245 list( $namespace, $fromKey, $from ) = $fromList;
279 - list( $namespace, $toKey, $to ) = $toList;
280246
281247 $dbr = wfGetDB( DB_SLAVE );
282248 $res = $dbr->select( 'page',
283249 array( 'page_namespace', 'page_title', 'page_is_redirect' ),
284250 array(
285251 'page_namespace' => $namespace,
286 - 'page_title >= ' . $dbr->addQuotes( $fromKey ),
287 - 'page_title <= ' . $dbr->addQuotes( $toKey ),
 252+ 'page_title >= ' . $dbr->addQuotes( $fromKey )
288253 ),
289254 __METHOD__,
290255 array(
291 - 'ORDER BY' => 'page_title ASC',
 256+ 'ORDER BY' => 'page_title',
292257 'LIMIT' => $this->maxPerPage + 1,
293258 'USE INDEX' => 'name_title',
294259 )
——@@ -326,15 +291,87 @@
327292 if ( $including ) {
328293 $out2 = '';
329294 } else {
330 - $nsForm = $this->namespaceForm( $namespace, $from, $to );
 295+ if( $from == '' ) {
 296+ // First chunk; no previous link.
 297+ $prevTitle = null;
 298+ } else {
 299+ # Get the last title from previous chunk
 300+ $dbr = wfGetDB( DB_SLAVE );
 301+ $res_prev = $dbr->select(
 302+ 'page',
 303+ 'page_title',
 304+ array( 'page_namespace' => $namespace, 'page_title < '.$dbr->addQuotes($from) ),
 305+ __METHOD__,
 306+ array( 'ORDER BY' => 'page_title DESC', 'LIMIT' => $this->maxPerPage, 'OFFSET' => ($this->maxPerPage - 1 ) )
 307+ );
 308+
 309+ # Get first title of previous complete chunk
 310+ if( $dbr->numrows( $res_prev ) >= $this->maxPerPage ) {
 311+ $pt = $dbr->fetchObject( $res_prev );
 312+ $prevTitle = Title::makeTitle( $namespace, $pt->page_title );
 313+ } else {
 314+ # The previous chunk is not complete, need to link to the very first title
 315+ # available in the database
 316+ $options = array( 'LIMIT' => 1 );
 317+ if ( ! $dbr->implicitOrderby() ) {
 318+ $options['ORDER BY'] = 'page_title';
 319+ }
 320+ $reallyFirstPage_title = $dbr->selectField( 'page', 'page_title', array( 'page_namespace' => $namespace ), __METHOD__, $options );
 321+ # Show the previous link if it s not the current requested chunk
 322+ if( $from != $reallyFirstPage_title ) {
 323+ $prevTitle = Title::makeTitle( $namespace, $reallyFirstPage_title );
 324+ } else {
 325+ $prevTitle = null;
 326+ }
 327+ }
 328+ }
 329+
 330+ $nsForm = $this->namespaceForm( $namespace, $from );
331331 $out2 = '<table style="background: inherit;" width="100%" cellpadding="0" cellspacing="0" border="0">';
332332 $out2 .= '<tr valign="top"><td>' . $nsForm;
333333 $out2 .= '</td><td align="' . $align . '" style="font-size: smaller; margin-bottom: 1em;">' .
334 - $sk->makeKnownLink( $wgContLang->specialPage( "Allpages" ), wfMsgHtml ( 'allpages' ) );
 334+ $sk->makeKnownLink( $wgContLang->specialPage( "Allpages" ),
 335+ wfMsgHtml ( 'allpages' ) );
 336+
 337+ $self = SpecialPage::getTitleFor( 'Allpages' );
 338+
 339+ # Do we put a previous link ?
 340+ if( isset( $prevTitle ) && $pt = $prevTitle->getText() ) {
 341+ $q = 'from=' . $prevTitle->getPartialUrl()
 342+ . ( $namespace ? '&namespace=' . $namespace : '' );
 343+ $prevLink = $sk->makeKnownLinkObj( $self,
 344+ wfMsgHTML( 'prevpage', htmlspecialchars( $pt ) ), $q );
 345+ $out2 .= ' | ' . $prevLink;
 346+ }
 347+
 348+ if( $n == $this->maxPerPage && $s = $dbr->fetchObject($res) ) {
 349+ # $s is the first link of the next chunk
 350+ $t = Title::MakeTitle($namespace, $s->page_title);
 351+ $q = 'from=' . $t->getPartialUrl()
 352+ . ( $namespace ? '&namespace=' . $namespace : '' );
 353+ $nextLink = $sk->makeKnownLinkObj( $self,
 354+ wfMsgHtml( 'nextpage', htmlspecialchars( $t->getText() ) ), $q );
 355+ $out2 .= ' | ' . $nextLink;
 356+ }
335357 $out2 .= "</td></tr></table><hr />";
336358 }
337359
338360 $wgOut->addHtml( $out2 . $out );
 361+ if( isset($prevLink) or isset($nextLink) ) {
 362+ $wgOut->addHtml( '<hr /><p style="font-size: smaller; float: ' . $align . '">' );
 363+ if( isset( $prevLink ) ) {
 364+ $wgOut->addHTML( $prevLink );
 365+ }
 366+ if( isset( $prevLink ) && isset( $nextLink ) ) {
 367+ $wgOut->addHTML( ' | ' );
 368+ }
 369+ if( isset( $nextLink ) ) {
 370+ $wgOut->addHTML( $nextLink );
 371+ }
 372+ $wgOut->addHTML( '</p>' );
 373+
 374+ }
 375+
339376 }
340377
341378 /**
——@@ -344,7 +381,7 @@
345382 * @static (sort of)
346383 * @access private
347384 */
348 -function getNamespaceKeyAndText( $ns, $text ) {
 385+function getNamespaceKeyAndText ($ns, $text) {
349386 if ( $text == '' )
350387 return array( $ns, '', '' ); # shortcut for common case
351388
——@@ -364,5 +401,4 @@
365402 return NULL;
366403 }
367404 }
368 -
369405 }
Index: trunk/phase3/languages/messages/MessagesEn.php
——@@ -2004,7 +2004,6 @@
20052005 'nextpage' => 'Next page ($1)',
20062006 'prevpage' => 'Previous page ($1)',
20072007 'allpagesfrom' => 'Display pages starting at:',
2008 -'allpagesto' => 'Display pages ending at:',
20092008 'allarticles' => 'All pages',
20102009 'allinnamespace' => 'All pages ($1 namespace)',
20112010 'allnotinnamespace' => 'All pages (not in $1 namespace)',

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r39416Rewrite allpages to use a "tree" of index range subpages (similar to the star...aaron17:55, 15 August 2008

Status & tagging log