From bc272498298816c3095dda19dc9602629590484b Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Thu, 9 May 2019 17:59:04 -0700
Subject: [PATCH] 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
---
 includes/changes/EnhancedChangesList.php | 55 +++++++++++++-----------
 includes/specials/SpecialWatchlist.php   |  3 ++
 2 files changed, 32 insertions(+), 26 deletions(-)

diff --git a/includes/changes/EnhancedChangesList.php b/includes/changes/EnhancedChangesList.php
index 81860590c3..8f32ba2007 100644
--- 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 1ef11b5ce7..56f5c8fdac 100644
--- 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;
-- 
2.20.1