Page MenuHomePhabricator

Mobile logout should not involve an interstitial
Closed, ResolvedPublic2 Estimated 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/

Event Timeline

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?

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 ?

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_WMF.

@alexhollender - any thoughts here?

For clarification are we talking about this screen?

en.m.wikipedia.org_w_index.php_title=Special_UserLogout&returnto=Comparison+of+functional+programming+languages&logoutToken=feb3ebebd55a208c99462c41177e0cd05da5d6db%2B%5C(iPhone 6_7_8).png (1×750 px, 66 KB)

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.

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 added a subscriber: ovasileva.

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

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

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

ovasileva claimed this task.

Thanks @pmiazga, resolving this now.