SpecialAllPages: Fix a few subtle "Previous page" link bugs
authorKevin Israel <pleasestand@live.com>
Sat, 10 Jan 2015 11:44:19 +0000 (06:44 -0500)
committerOri.livneh <ori@wikimedia.org>
Mon, 17 Aug 2015 07:20:15 +0000 (07:20 +0000)
* When finding the start of the previous page, exclude redirects if
  "Hide redirects" is checked. Otherwise, if some of the previous 345
  wiki pages were redirects, an equal number of results would be
  repeated when going back.
* Use the DB key form of the title (not the unprefixed text form)
  when building the query.
* Don't select unnecessary rows beyond the start of the previous page.
  At the same time, fix a bug that could cause results close to the
  beginning to be skipped when going back.
* When finding the start of the first page, don't omit ORDER BY for
  MySQL and SQLite.
* Don't suppress the link when the unprefixed title is "0" or similar
  or is numerically equal to the "from" parameter value.
* Suppress the link when the title would come after the "from"
  parameter value, not just when it would be equal to that value.

Change-Id: I2cc6958b894afa824a5345871b5a27b01f9fcc73

includes/specials/SpecialAllPages.php

index 74b1f7b..c4a67c0 100644 (file)
@@ -25,6 +25,7 @@
  * Implements Special:Allpages
  *
  * @ingroup SpecialPage
+ * @todo Rewrite using IndexPager
  */
 class SpecialAllPages extends IncludableSpecialPage {
 
@@ -179,6 +180,7 @@ class SpecialAllPages extends IncludableSpecialPage {
                $toList = $this->getNamespaceKeyAndText( $namespace, $to );
                $namespaces = $this->getContext()->getLanguage()->getNamespaces();
                $n = 0;
+               $prevTitle = null;
 
                if ( !$fromList || !$toList ) {
                        $out = $this->msg( 'allpagesbadtitle' )->parseAsBlock();
@@ -191,15 +193,13 @@ class SpecialAllPages extends IncludableSpecialPage {
                        list( , $toKey, $to ) = $toList;
 
                        $dbr = wfGetDB( DB_SLAVE );
-                       $conds = array(
-                               'page_namespace' => $namespace,
-                               'page_title >= ' . $dbr->addQuotes( $fromKey )
-                       );
-
+                       $filterConds = array( 'page_namespace' => $namespace );
                        if ( $hideredirects ) {
-                               $conds['page_is_redirect'] = 0;
+                               $filterConds['page_is_redirect'] = 0;
                        }
 
+                       $conds = $filterConds;
+                       $conds[] = 'page_title >= ' . $dbr->addQuotes( $fromKey );
                        if ( $toKey !== "" ) {
                                $conds[] = 'page_title <= ' . $dbr->addQuotes( $toKey );
                        }
@@ -234,6 +234,35 @@ class SpecialAllPages extends IncludableSpecialPage {
                        } else {
                                $out = '';
                        }
+
+                       if ( $fromKey !== '' && !$this->including() ) {
+                               # Get the first title from previous chunk
+                               $prevConds = $filterConds;
+                               $prevConds[] = 'page_title < ' . $dbr->addQuotes( $fromKey );
+                               $prevKey = $dbr->selectField(
+                                       'page',
+                                       'page_title',
+                                       $prevConds,
+                                       __METHOD__,
+                                       array( 'ORDER BY' => 'page_title DESC', 'OFFSET' => $this->maxPerPage - 1 )
+                               );
+
+                               if ( $prevKey === false ) {
+                                       # The previous chunk is not complete, need to link to the very first title
+                                       # available in the database
+                                       $prevKey = $dbr->selectField(
+                                               'page',
+                                               'page_title',
+                                               $prevConds,
+                                               __METHOD__,
+                                               array( 'ORDER BY' => 'page_title' )
+                                       );
+                               }
+
+                               if ( $prevKey !== false ) {
+                                       $prevTitle = Title::makeTitle( $namespace, $prevKey );
+                               }
+                       }
                }
 
                if ( $this->including() ) {
@@ -241,44 +270,6 @@ class SpecialAllPages extends IncludableSpecialPage {
                        return;
                }
 
-               if ( $from == '' ) {
-                       // First chunk; no previous link.
-                       $prevTitle = null;
-               } else {
-                       # Get the last title from previous chunk
-                       $dbr = wfGetDB( DB_SLAVE );
-                       $res_prev = $dbr->select(
-                               'page',
-                               'page_title',
-                               array( 'page_namespace' => $namespace, 'page_title < ' . $dbr->addQuotes( $from ) ),
-                               __METHOD__,
-                               array( 'ORDER BY' => 'page_title DESC',
-                                       'LIMIT' => $this->maxPerPage, 'OFFSET' => ( $this->maxPerPage - 1 )
-                               )
-                       );
-
-                       # Get first title of previous complete chunk
-                       if ( $dbr->numrows( $res_prev ) >= $this->maxPerPage ) {
-                               $pt = $dbr->fetchObject( $res_prev );
-                               $prevTitle = Title::makeTitle( $namespace, $pt->page_title );
-                       } else {
-                               # The previous chunk is not complete, need to link to the very first title
-                               # available in the database
-                               $options = array( 'LIMIT' => 1 );
-                               if ( !$dbr->implicitOrderby() ) {
-                                       $options['ORDER BY'] = 'page_title';
-                               }
-                               $reallyFirstPage_title = $dbr->selectField( 'page', 'page_title',
-                                       array( 'page_namespace' => $namespace ), __METHOD__, $options );
-                               # Show the previous link if it s not the current requested chunk
-                               if ( $from != $reallyFirstPage_title ) {
-                                       $prevTitle = Title::makeTitle( $namespace, $reallyFirstPage_title );
-                               } else {
-                                       $prevTitle = null;
-                               }
-                       }
-               }
-
                $self = $this->getPageTitle();
 
                $topLinks = array(
@@ -287,7 +278,7 @@ class SpecialAllPages extends IncludableSpecialPage {
                $bottomLinks = array();
 
                # Do we put a previous link ?
-               if ( $prevTitle && $pt = $prevTitle->getText() ) {
+               if ( $prevTitle ) {
                        $query = array( 'from' => $prevTitle->getText() );
 
                        if ( $namespace ) {
@@ -300,7 +291,7 @@ class SpecialAllPages extends IncludableSpecialPage {
 
                        $prevLink = Linker::linkKnown(
                                $self,
-                               $this->msg( 'prevpage', $pt )->escaped(),
+                               $this->msg( 'prevpage', $prevTitle->getText() )->escaped(),
                                array(),
                                $query
                        );