Page MenuHomePhabricator

Watch/unwatch url is incorrect for non-js users
Closed, ResolvedPublic2 Estimated Story PointsBUG REPORT

Description

Description:

When logged into beta with AMC on and Javascript Off, adding a page to the watchlist prompts the user to click yes to "Remove from watchlist." Removing a page from the watchlist prompts the user to "Add to watchlist".

For a filled star (watched) on an article the url points to https://en.m.wikipedia.beta.wmflabs.org/w/index.php?title=Qatar&action=watch but should point to action=unwatch

For an empty watchstar (unwatched) the url should be action=watch

BetaWatchListBug.gif (808×372 px, 1 MB)

Steps to Reproduce:

Navigate to a page on beta and click the watchstar icon.

Actual Results:

Clicking on the watchstar of a watched page prompts to be added to the watchlist.
Clicking on the watchstar of an un-watched page prompts to be removed from the watchlist.

Expected Results:

If the page was not on the watchlist it should prompt to be added.
If the page was on the watchlist it should prompt to be removed.

developer notes

The logic for creating the url is wrong and needs to be flipped.

	protected function createWatchPageAction(): IMenuEntry {
		$title = $this->title;
		$user = $this->user;
		$isWatched = $title && $user->isLoggedIn() && $user->isWatched( $title );
		$mode = $isWatched ? 'watch' : 'unwatch';

QA steps

Test on beta cluster e.g. https://en.m.wikipedia.beta.wmflabs.org/w/index.php?title=Qatar

Event Timeline

Edtadros renamed this task from Add/Remove watchlist switched in Beta with Logged in, AMC-ON, JS Off to WIP [Bug] Add/Remove watchlist switched in Beta with Logged in, AMC-ON, JS Off.Jul 29 2019, 1:43 PM
Edtadros renamed this task from WIP [Bug] Add/Remove watchlist switched in Beta with Logged in, AMC-ON, JS Off to [Bug] Add/Remove watchlist switched in Beta with Logged in, AMC-ON, JS Off.Jul 29 2019, 2:37 PM
Jdlrobson renamed this task from [Bug] Add/Remove watchlist switched in Beta with Logged in, AMC-ON, JS Off to Watch/unwatch url is incorrect for non-js users.Jul 29 2019, 8:22 PM
Jdlrobson updated the task description. (Show Details)
Jdlrobson moved this task from Incoming to Needs Prioritization on the Web-Team-Backlog board.
Jdlrobson moved this task from Needs Prioritization to Upcoming on the Web-Team-Backlog board.
Jdlrobson updated the task description. (Show Details)
Jdlrobson subscribed.

Nice catch Edward but not AMC specific! :-S

Jdlrobson set the point value for this task to 1.Jul 30 2019, 4:09 PM
Jdlrobson changed the point value for this task from 1 to 2.Jul 30 2019, 4:15 PM

Change 526515 had a related patch set uploaded (by Pmiazga; owner: Pmiazga):
[mediawiki/skins/MinervaNeue@master] Set correct action for watchstar link

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

Change 526515 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Set correct action for watchstar link

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

Test Result

Status: ✅ PASS
OS: macOS Mojave
Browser: Chrome
Device: MBP
Emulated Device: iPhoneX

Test Artifact(s):

QA steps
Test on beta cluster e.g. https://en.m.wikipedia.beta.wmflabs.org/w/index.php?title=Qatar
Navigate to a page on beta and click the watchstar icon.

Expected Results:
✅ AC1: If the page was not on the watchlist it should prompt to be added.
✅ AC2: If the page was on the watchlist it should prompt to be removed.

T229228.gif (621×286 px, 464 KB)

I added Bug: T229716 to address the confirmation watchstar link behavior.