Page MenuHomePhabricator

[Mobile regression] Patrol link does not show up on diffs in diffonly mode
Closed, ResolvedPublicBUG REPORT

Description

Steps to replicate the issue (include links if applicable):

  • Visit a page which can be patrolled
  • Append &diffonly=1 to the URL

What happens?:
No patrol link at bottom of page

Screenshot 2024-02-10 at 10.42.08 AM.png (442×2 px, 49 KB)

What should have happened instead?:

Screenshot 2024-02-10 at 10.41.49 AM.png (226×1 px, 28 KB)

This is particularly problematic on mobile where the diffonly mode is the default.

Software version (skip for WMF-hosted wikis like Wikipedia):

Other information (browser name/version, screenshots, etc.):

Event Timeline

Raised in T357202.
@Titore it seems the patrol link does not show in diffonly mode. In mean time you can change the URL to be diffonly=0 and it should show again. Let's continue the conversation here. cc @Nardog

The status quo is the desired behavior. You shouldn't be marking pages you haven't seen in parsed whole as patrolled.

@Nardog in mobile [[ https://gerrit.wikimedia.org/g/mediawiki/extensions/MobileFrontend/+/838a8138dd008ba8bca3f0de3f7407defacd850e/includes/specials/SpecialMobileDiff.php#194 | the patrol link has always been shown without the preview ]]and I'm not aware of any problems caused by that, so I think this desires a rethink. The following code change would fix this:

diff --git a/includes/diff/DifferenceEngine.php b/includes/diff/DifferenceEngine.php
index cd50cf07132..d9210f76180 100644
--- a/includes/diff/DifferenceEngine.php
+++ b/includes/diff/DifferenceEngine.php
@@ -1028,6 +1028,12 @@ class DifferenceEngine extends ContextSource {
 			if ( !$diffOnly ) {
 				$this->renderNewRevision();
 			}
+
+			// Allow extensions to optionally not show the final patrolled link
+			if ( $this->hookRunner->onDifferenceEngineRenderRevisionShowFinalPatrolLink() ) {
+				# Add redundant patrol link on bottom...
+				$out->addHTML( $this->markPatrolledLink() );
+			}
 		}
 	}
 
@@ -1260,12 +1266,6 @@ class DifferenceEngine extends ContextSource {
 				}
 			}
 		}
-
-		// Allow extensions to optionally not show the final patrolled link
-		if ( $this->hookRunner->onDifferenceEngineRenderRevisionShowFinalPatrolLink() ) {
-			# Add redundant patrol link on bottom...
-			$out->addHTML( $this->markPatrolledLink() );
-		}
 	}
 
 	/**
Jdlrobson renamed this task from Patrol link does not show up on diffs in diffonly mode to [Mobile regression] Patrol link does not show up on diffs in diffonly mode.Feb 10 2024, 6:50 PM
Jdlrobson added a project: Regression.

Change 1002615 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] Diffs: Always render patrol link in HTML and hide via CSS

https://gerrit.wikimedia.org/r/1002615

Jdlrobson triaged this task as Medium priority.Feb 12 2024, 8:17 PM

The status quo is the desired behavior. You shouldn't be marking pages you haven't seen in parsed whole as patrolled.

The status quo on mobile differed from the status quo on desktop. This was an unintentional change and we heard about it in short order, so treating it as a regression to be fixed in the near term is reasonable.

I'm not sure why the two experiences differed in this particular way. If it seems wrong, I think the next step is to investigate why they were different in the first place.

Change 1002627 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/MinervaNeue@master] Always show patrol link on Minerva skin

https://gerrit.wikimedia.org/r/1002627

Change 1002627 merged by jenkins-bot:

[mediawiki/skins/MinervaNeue@master] Always show patrol link on Minerva skin

https://gerrit.wikimedia.org/r/1002627

Change 1002615 merged by jenkins-bot:

[mediawiki/core@master] Diffs: Always render patrol link in HTML and hide via CSS

https://gerrit.wikimedia.org/r/1002615

@Titore this should be working again now. Could you confirm?

Jdlrobson claimed this task.
Jdlrobson added a project: Verified.

Thanks for verifying and sorry for the disruption to your workflow!