Page MenuHomePhabricator

[mobile arwiki] Suggested articles with pending revision will open in diff view on mobile
Closed, ResolvedPublic

Description

Steps to reproduce:

  1. On mobile arwiki wmf.19 select on Special:Homepage the following topic/task type: Physics + Update articles. The only one result will be returned: https://ar.m.wikipedia.org/w/index.php?title=لونا-غلوب&gesuggestededit=1
  2. Go to the article - the article will present the diff view of the pending revision:
Screen Shot 2022-07-07 at 3.07.03 PM.png (1×786 px, 192 KB)
Screen Shot 2022-07-07 at 3.37.42 PM.png (1×780 px, 176 KB)
  1. Clicking on the Edit - the editing mode will be presented as expected.

Event Timeline

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

As far as I know, we don't store the flagged revision status in the search index. We could add a filter to check the FlaggedRevision status of an article, though. @KStoller-WMF do you think this is important to prioritize? @Trizek-WMF do you have an idea of the frequency that newcomers might encounter this?

This task is about mobile, but the UX for desktop doesn't seem ideal either.

@Trizek-WMF do you have an idea of the frequency that newcomers might encounter this?

Frankly, I don't know; I can guess We have a high usage of mobile at Arabic Wikipedia, and a few wikis that have FR enabled.

In any case, no newcomers is supposed to get a diff as the result of a query for a task to work on. Maybe we should exclude articles with pending edits from the list of suggested tasks.

@Trizek-WMF do you have an idea of the frequency that newcomers might encounter this?

Frankly, I don't know; I can guess We have a high usage of mobile at Arabic Wikipedia, and a few wikis that have FR enabled.

In any case, no newcomers is supposed to get a diff as the result of a query for a task to work on. Maybe we should exclude articles with pending edits from the list of suggested tasks.

Cool. Filed the specific task for that as T312639: Suggested edits: Filter out articles that have pending edits from FlaggedRevisions.

The fact that the diff doesn’t collapse like on desktop is FlaggedRevs’ bug (the necessary ResourceLoader module is not marked to be mobile-friendly). However, I think filtering out articles with pending changes is a good idea regardless: the more pending changes there are, the less likely that the new edit will be reviewed in a timely manner, which is discouraging for newcomers.

I know that this doesn't only relate to "add a link" structured tasks, but I think this and T312639 seem like a relevant improvements that we might be able to work on as we start to work on that epic. (Especially since Arabic Wikipedia has been vocal about wanting more "add a link" improvements.)

Here is my humble 2c, FR is one of oldest extensions we have in production that didn't get much attention since deployment and has been the cause for trouble and issues since then. See T185664: Code stewardship review: FlaggedRevs (and subtickets)

One of the biggest issues of FR that I hoped to address in T277883 is that the frontend messages can take four different shapes depending on wiki's configuration. It has low-profile mode and high-profile mode. And has SimpleUI and non-SimpleUI (and fun fact, the non-simple one is actually simpler) and you get a different UI depending on combination of each of these (plus stuff like a sentence being repeated twice: T277883#7073260).

The frontend is in dire need of simplification and improvement, if UX by growth team can take a look and come up with some recommendations which I would implement, it'd be greatly appreciated

Thanks for the context, @Ladsgroup !

We'll certainly try to address FR quirks that relate to Growth tools, but realistically I'm not sure the Growth team will have time to prioritize an in-depth look at Flagged Revisions. Our regular UX designer is out on extended leave currently, so we have limited design support currently.

Perhaps eventually this is work that could be funneled to the Moderator Tools team or as a future Community Tech team project?

IIRC the diff only shows if you have reviewer right, so this has little impact on a feature meant for newcomers. @Tacsipacsi makes a good point though.

Hm, I can't reproduce this locally. I guess one of the many settings of FlaggedRevs influences whether the diff is shown during page views.

It’s not a configuration variable, just a user preference (flaggedrevsviewdiffs, “Show the pending changes diff when viewing the latest pending revision”, in the Recent changes section), which defaults to enabled on arwiki (T220186).

Thanks @Tacsipacsi! I can reproduce the issue locally with FlaggedRevs and MobileFrontend with

$wgFlaggedRevsLowProfile = true;
$wgSimpleFlaggedRevsUI = true;
$wgDefaultUserOptions['flaggedrevsviewdiffs'] = 1;

The two desktop-only modules have the following functionality:

  1. ext.flaggedRevs.advanced:
    1. Handle the hover behavior for the little dropdown box with the eye or stop icon in the upper right corner of the article. No-op on mobile which doesn't show the box in the first place. (Probably a bug, although if it did show, it would probably need to be adopted to touch devices.)
    2. Collapse the diff and show an uncollapse button - what this bug is about.
    3. Similarly hide/unhide a log snippet (FlaggedRevsXML::stabilityLogExcerpt()), shown on pages with manual stability settings, probably? I couldn't figure out how to get this to actually show.
    4. Change the text of the edit form's save button (from "Submit changes" to "Save change", which is somewhat pointless, but with better wording this seems like a useful thing) when the option to also review all pending edits is checked. Doesn't affect mobile since that's not using the standard edit form. Should probably be in the other module?
  2. ext.flaggedRevs.review:
    1. Make the review form submit via AJAX.
    2. Manage enabled state of the review form's submit button depending on whether submitting would be a noop. Mildly broken (only executed on page load, not after every change to the form).
    3. Something logic for multi-dimensional reviews (option colors + disallowing setting any dimension to level 0) that I think was broken completely by rEFLR3fe0bacc5bb3: Support only one dimension.

Most of these seem useful to me on mobile and they should be enabled, except 1.A and 1.D which don't do anything on mobile. ext.flaggedRevs.advanced is loaded on all page views and is 3.1K minified, 1.1K minified + gzipped. ext.flaggedRevs.review is loaded on all page views but only if the user has review rights; the two together are 8.7K minified, 2.6K minified + gzipped (probably less in practice since they are bundled with other files for gzipping). So I think that's acceptable.

IIRC the diff only shows if you have reviewer right, so this has little impact on a feature meant for newcomers.

That was incorrect. It does get shown to all users, regardless of whether they have the right to do something about it, including anonymous users if they are looking at the latest revision of the page (which they by default aren't). I guess that makes sense as a way of informing readers about un-vetted changes.

Change 817927 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/extensions/FlaggedRevs@master] Enable ext.flaggedRevs.advanced and ext.flaggedRevs.review on mobile

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

Change 817932 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/extensions/FlaggedRevs@master] Rearrange and cull review-related JS

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

Something logic for multi-dimensional reviews (option colors + disallowing setting any dimension to level 0) that I think was broken completely by rEFLR3fe0bacc5bb3: Support only one dimension.

Not completely, it still serves to disable the Accept button when level 0 is selected. Dropped the easily droppable part in a follow-up.

Tgr edited projects, added Growth-Team (Current Sprint); removed Growth-Team.
Tgr moved this task from Incoming to Code Review on the Growth-Team (Current Sprint) board.

Change 817927 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@master] Enable ext.flaggedRevs.advanced and ext.flaggedRevs.review on mobile

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

On https://de.m.wikipedia.beta.wmflabs.org/wiki/FRtest, the diff is collapsible for me (I seem to have reviewer rights there), but the CSS for coloring the whole notice and the [checked revision]/[pending revision] tags doesn’t load for some reason I was unable to figure out.

IIRC the diff only shows if you have reviewer right, so this has little impact on a feature meant for newcomers.

That was incorrect. It does get shown to all users, regardless of whether they have the right to do something about it, including anonymous users if they are looking at the latest revision of the page (which they by default aren't). I guess that makes sense as a way of informing readers about un-vetted changes.

This sounds like a bug—the preference is available only to reviewers, so in this case it not only defaults to enabled on arwiki, it’s forced to enabled for non-reviewers. (Since this default preference is relatively new, I suppose no one tested this use case formerly.) Maybe the easiest fix is to simply allow anyone to change this preference—this is the smallest change to the status quo, and it also reduces the code complexity (in contrast to the other possible solution, namely that the code responsible for displaying the diff also checks for permissions in addition to the preference, not only the preferences code, which would increase code complexity).

Change 817932 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@master] Rearrange and cull review-related JS

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

the CSS for coloring the whole notice and the [checked revision]/[pending revision] tags doesn’t load for some reason I was unable to figure out.

That was removed in rEFLR7d70c7bf25d1: Drop coloring of the radio select options.. Or do you mean the page history colors? I think the mobile history is too different for those to work.

This sounds like a bug—the preference is available only to reviewers, so in this case it not only defaults to enabled on arwiki, it’s forced to enabled for non-reviewers. (Since this default preference is relatively new, I suppose no one tested this use case formerly.) Maybe the easiest fix is to simply allow anyone to change this preference—this is the smallest change to the status quo, and it also reduces the code complexity (in contrast to the other possible solution, namely that the code responsible for displaying the diff also checks for permissions in addition to the preference, not only the preferences code, which would increase code complexity).

Preferences are always forced on readers, so if we don't consider it reasonable, it shouldn't be the default.

Tested on arwiki wmf.23 - the diff for a flagged revision is briefly shown when a page loads; it's more noticeable on mobile and on slow connection (see the gif below).

flash_flagged_diff2.gif (969×577 px, 1 MB)

It is not specific for the case when a user is redirected to a page via Special:Homepage SE module. Go to, e.g. https://ar.wikipedia.org/wiki/لونا-غلوب and reload the page - the diff will be briefly shown. Or enter in the Search field لونا-غلوب and click on a suggestion - the diff will be briefly displayed.

Closing the task as Resolved - the reported issue was resolved. The issue with showing diff is not specific to Growth features and is not interfering with user experience.

That was removed in rEFLR7d70c7bf25d1: Drop coloring of the radio select options.. Or do you mean the page history colors? I think the mobile history is too different for those to work.

I mean the colours in the diff view that is discussed here. Compare:

DesktopMobile
Screenshot 2022-08-06 at 17-50-16 لونا-غلوب - ويكيبيديا.png (90×1 px, 11 KB)
Screenshot 2022-08-06 at 17-50-36 لونا-غلوب - ويكيبيديا.png (78×993 px, 24 KB)

Preferences are always forced on readers, so if we don't consider it reasonable, it shouldn't be the default.

First, logged-in users are always able to change their preferences—some may even register for the sole purpose of being able to change their preferences.

Second, since the preference is unavailable for non-reviewers, I think we can reasonably assume that arwiki users voting on this did not consider the effect of this change on non-reviewers (especially as the request contained preference names, not preference IDs, so they probably didn’t look at the code), and eventual other communities asking to change this preference will also assume that this doesn’t affect non-reviewers. Since no one complained from arwiki, they’re probably okay with the status quo, so IMO it’s better to provide the preference to more users but not change it for anyone than suddenly disabling it for non-reviewers without an opt-in.

Closing the task as Resolved - the reported issue was resolved.

My suggestion in T312603#8065336 wasn’t considered:

However, I think filtering out articles with pending changes is a good idea regardless: the more pending changes there are, the less likely that the new edit will be reviewed in a timely manner, which is discouraging for newcomers.

It’s okay for me if you decide it’s not worth the complexity, but I’d like you to decide so, not just let it happen that way.

I mean the colours in the diff view that is discussed here. Compare:

DesktopMobile
Screenshot 2022-08-06 at 17-50-16 لونا-غلوب - ويكيبيديا.png (90×1 px, 11 KB)
Screenshot 2022-08-06 at 17-50-36 لونا-غلوب - ويكيبيديا.png (78×993 px, 24 KB)

I see, thanks for the screenshots. The relevant styles weren't used for Minerva. I don't have the bandwidth to track down which CSS style does exactly what, but https://gerrit.wikimedia.org/r/c/mediawiki/extensions/FlaggedRevs/+/821594 seems to fix the background issue.

It’s okay for me if you decide it’s not worth the complexity, but I’d like you to decide so, not just let it happen that way.

That's T312639: Suggested edits: Filter out articles that have pending edits from FlaggedRevisions.