Merge "Fix order on Special:Contributions when timestamps are identical"

diff --combined includes/specials/pagers/ContribsPager.php
index 510bf92,90a57ff..2bfc923
--- 1/includes/specials/pagers/ContribsPager.php
--- 2/includes/specials/pagers/ContribsPager.php
+++ b/includes/specials/pagers/ContribsPager.php
@@@ -87,10 -87,6 +87,6 @@@ class ContribsPager extends RangeChrono
                }
                $this->getDateRangeCond( $startTimestamp, $endTimestamp );
  
-               // This property on IndexPager is set by $this->getIndexField() in parent::__construct().
-               // We need to reassign it here so that it is used when the actual query is ran.
-               $this->mIndexField = $this->getIndexField();
- 
                // Most of this code will use the 'contributions' group DB, which can map to replica DBs
                // with extra user based indexes or partioning by user. The additional metadata
                // queries should use a regular replica DB since the lookup pattern is not all by user.
@@@ -212,6 -208,22 +208,22 @@@
                        $ipRangeConds = $user->isAnon() ? $this->getIpRangeConds( $this->mDb, $this->target ) : null;
                        if ( $ipRangeConds ) {
                                $queryInfo['tables'][] = 'ip_changes';
+                               /**
+                                * These aliases make `ORDER BY rev_timestamp, rev_id` from {@see getIndexField} and
+                                * {@see getExtraSortFields} use the replicated `ipc_rev_timestamp` and `ipc_rev_id`
+                                * columns from the `ip_changes` table, for more efficient queries.
+                                * @see https://phabricator.wikimedia.org/T200259#4832318
+                                */
+                               $queryInfo['fields'] = array_merge(
+                                       [
+                                               'rev_timestamp' => 'ipc_rev_timestamp',
+                                               'rev_id' => 'ipc_rev_id',
+                                       ],
+                                       array_diff( $queryInfo['fields'], [
+                                               'rev_timestamp',
+                                               'rev_id',
+                                       ] )
+                               );
                                $queryInfo['join_conds']['ip_changes'] = [
                                        'LEFT JOIN', [ 'ipc_rev_id = rev_id' ]
                                ];
@@@ -350,17 -362,19 +362,19 @@@
        }
  
        /**
-        * Override of getIndexField() in IndexPager.
-        * For IP ranges, it's faster to use the replicated ipc_rev_timestamp
-        * on the `ip_changes` table than the rev_timestamp on the `revision` table.
-        * @return string Name of field
+        * @return string
         */
        public function getIndexField() {
-               if ( $this->isQueryableRange( $this->target ) ) {
-                       return 'ipc_rev_timestamp';
-               } else {
-                       return 'rev_timestamp';
-               }
+               // Note this is run via parent::__construct() *before* $this->target is set!
+               return 'rev_timestamp';
+       }
+ 
+       /**
+        * @return string[]
+        */
+       protected function getExtraSortFields() {
+               // Note this is run via parent::__construct() *before* $this->target is set!
+               return [ 'rev_id' ];
        }
  
        function doBatchLookups() {
@@@ -415,10 -429,9 +429,10 @@@
         * id then null is returned.
         *
         * @param object $row
 +       * @param Title|null $title
         * @return Revision|null
         */
 -      public function tryToCreateValidRevision( $row ) {
 +      public function tryToCreateValidRevision( $row, $title = null ) {
                /*
                 * There may be more than just revision rows. To make sure that we'll only be processing
                 * revisions here, let's _try_ to build a revision out of our row (without displaying
@@@ -428,7 -441,7 +442,7 @@@
                 */
                Wikimedia\suppressWarnings();
                try {
 -                      $rev = new Revision( $row );
 +                      $rev = new Revision( $row, 0, $title );
                        $validRevision = (bool)$rev->getId();
                } catch ( Exception $e ) {
                        $validRevision = false;
@@@ -456,16 -469,11 +470,16 @@@
  
                $linkRenderer = MediaWikiServices::getInstance()->getLinkRenderer();
  
 -              $rev = $this->tryToCreateValidRevision( $row );
 +              $page = null;
 +              // Create a title for the revision if possible
 +              // Rows from the hook may not include title information
 +              if ( isset( $row->page_namespace ) && isset( $row->page_title ) ) {
 +                      $page = Title::newFromRow( $row );
 +              }
 +              $rev = $this->tryToCreateValidRevision( $row, $page );
                if ( $rev ) {
                        $attribs['data-mw-revid'] = $rev->getId();
  
 -                      $page = Title::newFromRow( $row );
                        $link = $linkRenderer->makeLink(
                                $page,
                                $page->getPrefixedText(),
@@@ -531,7 -539,7 +545,7 @@@
                        }
  
                        $lang = $this->getLanguage();
 -                      $comment = $lang->getDirMark() . Linker::revComment( $rev, false, true );
 +                      $comment = $lang->getDirMark() . Linker::revComment( $rev, false, true, false );
                        $date = $lang->userTimeAndDate( $row->rev_timestamp, $user );
                        if ( $rev->userCan( Revision::DELETED_TEXT, $user ) ) {
                                $d = $linkRenderer->makeKnownLink(
@@@ -574,17 -582,10 +588,17 @@@
                                $del .= ' ';
                        }
  
 -                      $diffHistLinks = Html::rawElement( 'ul',
 -                              [ 'class' => 'mw-changeslist-link-list' ],
 -                              Html::rawElement( 'li', [], $difftext ) .
 -                              Html::rawElement( 'li', [], $histlink )
 +                      // While it might be tempting to use a list here
 +                      // this would result in clutter and slows down navigating the content
 +                      // in assistive technology.
 +                      // See https://phabricator.wikimedia.org/T205581#4734812
 +                      $diffHistLinks = Html::rawElement( 'span',
 +                              [ 'class' => 'mw-changeslist-links' ],
 +                              // The spans are needed to ensure the dividing '|' elements are not
 +                              // themselves styled as links.
 +                              Html::rawElement( 'span', [], $difftext ) .
 +                              ' ' . // Space needed for separating two words.
 +                              Html::rawElement( 'span', [], $histlink )
                        );
  
                        # Tags, if any.