watchlist: fix nonsensical timestamp/boolean comparisons in EnhancedRecentChanges

Also fix bug where the "changes since last visit" counter failed to show for pages
where their entire history is still in recent changes. Although the "new" flag is
set for the RCCacheEntry block, there can still be multiple revisions since the
last one seen by the user. This change accounts for that case.

Bug: T218511
Change-Id: I92060bd26d8642937cad7f8c1ace3c5e066790be

diff --git a/includes/changes/EnhancedChangesList.php b/includes/changes/EnhancedChangesList.php
index 8186059..8f32ba2 100644 (file)
--- a/includes/changes/EnhancedChangesList.php
+++ b/includes/changes/EnhancedChangesList.php
@@ -184,9 +184,7 @@ class EnhancedChangesList extends ChangesList {
                        $tableClasses[] = Sanitizer::escapeClass( 'mw-changeslist-ns'
                                . $block[0]->mAttribs['rc_namespace'] . '-' . $block[0]->mAttribs['rc_title'] );
                }
-               if ( $block[0]->watched
-                       && $block[0]->mAttribs['rc_timestamp'] >= $block[0]->watched
-               ) {
+               if ( $block[0]->watched ) {
                        $tableClasses[] = 'mw-changeslist-line-watched';
                } else {
                        $tableClasses[] = 'mw-changeslist-line-not-watched';
@@ -219,7 +217,7 @@ class EnhancedChangesList extends ChangesList {
                foreach ( $block as $rcObj ) {
                        // If all log actions to this page were hidden, then don't
                        // give the name of the affected page for this block!
-                       if ( !$this->isDeleted( $rcObj, LogPage::DELETED_ACTION ) ) {
+                       if ( !static::isDeleted( $rcObj, LogPage::DELETED_ACTION ) ) {
                                $namehidden = false;
                        }
                        $u = $rcObj->userlink;
@@ -260,7 +258,8 @@ class EnhancedChangesList extends ChangesList {
                } elseif ( $allLogs ) {
                        $articleLink = $this->maybeWatchedLink( $block[0]->link, $block[0]->watched );
                } else {
-                       $articleLink = $this->getArticleLink( $block[0], $block[0]->unpatrolled, $block[0]->watched );
+                       $articleLink = $this->getArticleLink(
+                               $block[0], $block[0]->unpatrolled, $block[0]->watched );
                }
 
                $queryParams['curid'] = $curId;
@@ -386,9 +385,7 @@ class EnhancedChangesList extends ChangesList {
                $lineParams = [ 'targetTitle' => $rcObj->getTitle() ];
 
                $classes = [ 'mw-enhanced-rc' ];
-               if ( $rcObj->watched
-                       && $rcObj->mAttribs['rc_timestamp'] >= $rcObj->watched
-               ) {
+               if ( $rcObj->watched ) {
                        $classes[] = 'mw-enhanced-watched';
                }
                $classes = array_merge( $classes, $this->getHTMLClasses( $rcObj, $rcObj->watched ) );
@@ -421,7 +418,7 @@ class EnhancedChangesList extends ChangesList {
                                [],
                                $params
                        );
-                       if ( $this->isDeleted( $rcObj, Revision::DELETED_TEXT ) ) {
+                       if ( static::isDeleted( $rcObj, Revision::DELETED_TEXT ) ) {
                                $link = '<span class="history-deleted">' . $link . '</span> ';
                        }
                }
@@ -503,7 +500,7 @@ class EnhancedChangesList extends ChangesList {
        /**
         * Generates amount of changes (linking to diff ) & link to history.
         *
-        * @param array $block
+        * @param RCCacheEntry[] $block
         * @param array $queryParams
         * @param bool $allLogs
         * @param bool $isnew
@@ -529,7 +526,7 @@ class EnhancedChangesList extends ChangesList {
                /** @var RCCacheEntry $rcObj */
                foreach ( $block as $rcObj ) {
                        // Same logic as below inside main foreach
-                       if ( $rcObj->watched && $rcObj->mAttribs['rc_timestamp'] >= $rcObj->watched ) {
+                       if ( $rcObj->watched ) {
                                $sinceLast++;
                                $unvisitedOldid = $rcObj->mAttribs['rc_last_oldid'];
                        }
@@ -552,9 +549,10 @@ class EnhancedChangesList extends ChangesList {
                $block0 = $block[0];
                $last = $block[count( $block ) - 1];
                if ( !$allLogs ) {
-                       if ( !ChangesList::userCan( $rcObj, Revision::DELETED_TEXT, $this->getUser() ) ||
+                       if (
                                $isnew ||
-                               $rcObj->mAttribs['rc_type'] == RC_CATEGORIZE
+                               $rcObj->mAttribs['rc_type'] == RC_CATEGORIZE ||
+                               !ChangesList::userCan( $rcObj, Revision::DELETED_TEXT, $this->getUser() )
                        ) {
                                $links['total-changes'] = Html::rawElement( 'span', [], $nchanges[$n] );
                        } else {
@@ -569,19 +567,24 @@ class EnhancedChangesList extends ChangesList {
                                                ]
                                        )
                                );
-                               if ( $sinceLast > 0 && $sinceLast < $n ) {
-                                       $links['total-changes-since-last'] = Html::rawElement( 'span', [],
-                                               $this->linkRenderer->makeKnownLink(
-                                                       $block0->getTitle(),
-                                                       new HtmlArmor( $sinceLastVisitMsg[$sinceLast] ),
-                                                       [ 'class' => 'mw-changeslist-groupdiff' ],
-                                                       $queryParams + [
-                                                               'diff' => $currentRevision,
-                                                               'oldid' => $unvisitedOldid,
-                                                       ]
-                                               )
-                                       );
-                               }
+                       }
+
+                       if (
+                               $rcObj->mAttribs['rc_type'] != RC_CATEGORIZE &&
+                               $sinceLast > 0 &&
+                               $sinceLast < $n
+                       ) {
+                               $links['total-changes-since-last'] = Html::rawElement( 'span', [],
+                                       $this->linkRenderer->makeKnownLink(
+                                               $block0->getTitle(),
+                                               new HtmlArmor( $sinceLastVisitMsg[$sinceLast] ),
+                                               [ 'class' => 'mw-changeslist-groupdiff' ],
+                                               $queryParams + [
+                                                       'diff' => $currentRevision,
+                                                       'oldid' => $unvisitedOldid,
+                                               ]
+                                       )
+                               );
                        }
                }
 

diff --git a/includes/specials/SpecialWatchlist.php b/includes/specials/SpecialWatchlist.php
index 1ef11b5..56f5c8f 100644 (file)
--- a/includes/specials/SpecialWatchlist.php
+++ b/includes/specials/SpecialWatchlist.php
@@ -554,6 +554,9 @@ class SpecialWatchlist extends ChangesListSpecialPage {
                                $rc->numberofWatchingusers = 0;
                        }
 
+                       // XXX: this treats pages with no unseen changes as "not on the watchlist" since
+                       // everything is on the watchlist and it is an easy way to make pages with unseen
+                       // changes appear bold. @TODO: clean this up.
                        $changeLine = $list->recentChangesLine( $rc, $unseen, $counter );
                        if ( $changeLine !== false ) {
                                $s .= $changeLine;