Page MenuHomePhabricator

Mobile logout should not involve an interstitial
Closed, ResolvedPublic2 Estimate Story Points

Description

Without Javascript, clicking on the logout link on desktop will take you to an interstitial where you have to press a button to log out. (This is to avoid writes on a GET request, and to some extent for CSRF security.) With Javascript, the logout happens via an AJAX request (added in T222626) and the user does not see the interstitial.

For mobile the same thing was not done - clicking logout takes the user to the interstitial (regardless of Javascript being enabled/disabled), which is not great UX.

Developer notes

Per the direction in T232734#5501722 we will copy/pasta the code from core into Minerva.

Code that has to be used here can be found here: https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/506386/

Details

Related Gerrit Patches:

Event Timeline

Tgr created this task.Sep 12 2019, 12:00 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 12 2019, 12:00 PM
Jdlrobson added subscribers: Ladsgroup, Jdlrobson.

Looks like the code uses mediawiki.page.ready which is not loaded on mobile for various reasons. @Ladsgroup Can that code be made available in its own dedicated module?

Jdlrobson moved this task from Incoming to Needs Prioritization on the Readers-Web-Backlog board.

Looks like the code uses mediawiki.page.ready which is not loaded on mobile for various reasons. @Ladsgroup Can that code be made available in its own dedicated module?

We decided against putting the code in shared place given the overhead of registering new modules. The code is so short that copy-pasting it should not that bad. Thoughts @Krinkle ?

Krinkle added a comment.EditedSep 25 2019, 7:56 PM

Indeed, the code is a trivial 1-line API call. Should be fine for minerva to use directly in its own code, probably also easier to maintain that way. See T111565 for the larger task of sharing this module between desktop and mobile.

ovasileva triaged this task as Medium priority.Oct 15 2019, 2:22 PM
ovasileva added a subscriber: alexhollender.

@alexhollender - any thoughts here?

For clarification are we talking about this screen?

I am seeing this even with Javascript enabled (@Jdlrobson you once mentioned that this was due to something that was implemented during the Prague hackathon). I agree that (in both cases) we don't need a confirmation screen for logging out.

Jdlrobson added a comment.EditedOct 15 2019, 2:30 PM

This task is about removing that screen altogether by making logout a single click. This screen is a regression from a change in core that we've never prioritised and the fix is pretty straightforward.

This task is about removing that screen altogether by making logout a single click. This screen is a regression from a chance in core that we've never prioritised and the fix is pretty straightforward.

Sounds good.

I think this is one of our more damaging but easy to fix open bugs and im a bit worried it will be forgotten in triaged but future.

I think this is one of our more damaging but easy to fix open bugs and im a bit worried it will be forgotten in triaged but future.

Logout rates are actually pretty low, but if it's an easy fix let's do it.

ovasileva removed ovasileva as the assignee of this task.Oct 15 2019, 4:01 PM
ovasileva added a subscriber: ovasileva.
Jdlrobson updated the task description. (Show Details)Oct 15 2019, 5:33 PM
Restricted Application added a subscriber: Masumrezarock100. · View Herald TranscriptOct 15 2019, 5:33 PM
ovasileva set the point value for this task to 2.Oct 17 2019, 5:28 PM
pmiazga updated the task description. (Show Details)Oct 17 2019, 5:34 PM

Change 545392 had a related patch set uploaded (by Pmiazga; owner: Pmiazga):
[mediawiki/skins/MinervaNeue@master] Logout users via API Ajax call to prevent extra step on Special:Logout

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

pmiazga removed pmiazga as the assignee of this task.Oct 23 2019, 5:06 PM
pmiazga added a subscriber: pmiazga.
Krinkle removed a subscriber: Krinkle.Oct 25 2019, 12:26 AM

We are going to wait till @pmiazga gets back to do this one.

Jdlrobson assigned this task to pmiazga.Oct 30 2019, 5:13 PM

Change 549642 had a related patch set uploaded (by Pmiazga; owner: Pmiazga):
[mediawiki/skins/MinervaNeue@master] Hygiene: Drop optional parameters from SingleMenuEntry constructor

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

Change 549643 had a related patch set uploaded (by Pmiazga; owner: Pmiazga):
[mediawiki/skins/MinervaNeue@master] Hygiene: Remove PageActionMenuEntry

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

Change 549642 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Hygiene: Drop optional parameters from SingleMenuEntry constructor

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

Change 549643 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Hygiene: Remove PageActionMenuEntry

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

Change 545392 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Logout users via API Ajax call to prevent extra step on Special:Logout

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

Tested on Beta cluster - works as expected

pmiazga removed pmiazga as the assignee of this task.Dec 9 2019, 6:15 PM
ovasileva closed this task as Resolved.Dec 9 2019, 6:44 PM
ovasileva claimed this task.

Thanks @pmiazga, resolving this now.