Page MenuHomePhabricator
Create Task
Maniphest T232734

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

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:
mediawiki/skins/MinervaNeue : masterLogout users via API Ajax call to prevent extra step on Special:Logout
mediawiki/skins/MinervaNeue : masterHygiene: Remove PageActionMenuEntry
mediawiki/skins/MinervaNeue : masterHygiene: Drop optional parameters from SingleMenuEntry constructor

Related Objects

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 a project: Readers-Web-Backlog.Sep 17 2019, 1:09 AM
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 assigned this task to ovasileva.Sep 17 2019, 5:02 PM
Jdlrobson moved this task from Incoming to Needs Prioritization on the Readers-Web-Backlog board.
Ladsgroup added a subscriber: Krinkle.Sep 17 2019, 11:38 PM
In T232734#5497639, @Jdlrobson wrote:

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 moved this task from Needs Prioritization to Triaged but Future on the Readers-Web-Backlog board.
ovasileva added a subscriber: alexhollender.

@alexhollender - any thoughts here?

alexhollender added a comment.Oct 15 2019, 2:29 PM

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.

alexhollender added a comment.Oct 15 2019, 2:36 PM
In T232734#5575398, @Jdlrobson wrote:

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.

Jdlrobson moved this task from Triaged but Future to Needs Prioritization on the Readers-Web-Backlog board.Oct 15 2019, 3:15 PM

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.

ovasileva edited projects, added Readers-Web-Backlog (Kanbanana-2019-20-Q2); removed Readers-Web-Backlog.Oct 15 2019, 3:17 PM
In T232734#5575656, @Jdlrobson wrote:

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
ovasileva moved this task from Upcoming to Ready for Development on the Readers-Web-Backlog (Kanbanana-2019-20-Q2) board.
pmiazga updated the task description. (Show Details)Oct 17 2019, 5:34 PM
gerritbot added a comment.Oct 22 2019, 9:07 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 moved this task from Doing to Needs Code Review on the Readers-Web-Backlog (Kanbanana-2019-20-Q2) board.
pmiazga added a subscriber: pmiazga.
Krinkle removed a subscriber: Krinkle.Oct 25 2019, 12:26 AM
Jdlrobson added a comment.Oct 30 2019, 5:13 PM

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
gerritbot added a comment.Nov 7 2019, 9:50 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

gerritbot added a comment.Nov 7 2019, 9:51 PM

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

gerritbot added a comment.Nov 11 2019, 9:23 PM

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

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

gerritbot added a comment.Nov 26 2019, 10:39 PM

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

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

gerritbot added a comment.Nov 26 2019, 10:56 PM

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

pmiazga moved this task from Needs QA to Ready for Signoff on the Readers-Web-Backlog (Kanbanana-2019-20-Q2) board.Dec 9 2019, 6:14 PM

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.

Content licensed under Creative Commons Attribution-ShareAlike 3.0 (CC-BY-SA) unless otherwise noted; code licensed under GNU General Public License (GPL) or other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL