Page MenuHomePhabricator

MinervaNeue incorrectly passes data-mw=interface to watchstar even when it is not used for action=watch requests
Closed, ResolvedPublic

Description

MinervaNeue displays the watchstar unconditionally to all users, and either uses it as a watchlist client (by submitting action=watch requests) or as a CTA for signing up (see T330518). It also always includes data-mw=interface in said watchstar, even when it is not used as an actual watchlist client. This means MW Core tries to use it for watchlist requests, and shows an useless error message instead of the CTA draw.

This can be fixed by including data-mw=interface only when the button is supposed to be used for action=watch API. For more details, see https://gerrit.wikimedia.org/r/c/mediawiki/core/+/953717 (particularly the last comment).

Event Timeline

Restricted Application added subscribers: Masumrezarock100, Aklapper. · View Herald Transcript

Adding Minerva tags, as it affects Minerva. Will have a look soon, as this happened due to Growth's (my) changes to watchlist due to IP Masking.

Change 953717 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/core@master] mediawiki.page.watch.ajax: Do not init if user doesn't have watchlist permissions

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

Urbanecm_WMF added a subscriber: Jdrewniak.

I went with option 1.B, as user groups are already included on frontend (wgUserGroups), which might require a DB query to work properly. From there, getting the list of permissions is a configuration lookup. With this patch, MinervaNeue's "Please register" CTA on the watch icon/button should work from a temporary user account context. This fixes a regression from rMWe62fd3608f13: Allow watchlist UI for temp users with sufficient rights / T341976.

FYI @Jdrewniak, this is a follow-up fix to what got fixed in T344870: MinervaNeue: Watchstar missing for anonymous users.

Urbanecm_WMF changed the task status from Open to In Progress.Oct 16 2023, 4:06 PM

Change 967560 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/skins/MinervaNeue@master] Do not use data-mw=interface when watchstar is used as a CTA

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

Urbanecm_WMF renamed this task from mediawiki.page.watch.ajax incorrectly assumes all users have watchlist access to MinervaNeue incorrectly passes data-mw=interface to watchstar even when it is not used for action=watch requests.Oct 21 2023, 3:46 PM
Urbanecm_WMF updated the task description. (Show Details)

Change 953717 abandoned by Urbanecm:

[mediawiki/core@master] watchlist: Do not init watch button when lacking permissions

Reason:

in favor of Id9d6d9e7394b52d11ac6ce0b7d33319b3a761789

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

Moving back to incoming on Web's board, as this now needs a MinervaNeue patch to be reviewed (https://gerrit.wikimedia.org/r/c/mediawiki/skins/MinervaNeue/+/967560).

Change 967560 merged by jenkins-bot:

[mediawiki/skins/MinervaNeue@master] Do not use data-mw=interface when watchstar is used as a CTA

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

Urbanecm_WMF changed the task status from In Progress to Open.Oct 23 2023, 10:14 PM
Urbanecm_WMF moved this task from Code Review to QA on the Growth-Team (Sprint 1 (Growth Team)) board.
Etonkovidova subscribed.

Checked for possible regression and for watchlist add/remove behavior for temp users - all looks as expected.
Temp users (tested in cswiki beta) have the icon displayed which upon clicking will present the same message as for anon users:

Screen Shot 2023-10-24 at 12.54.44 PM.png (1×796 px, 212 KB)