[Lock screen] Always show "Switch User" button regardless of whether fake "switch user" item is shown
ClosedPublic

Authored by ngraham on Mon, Aug 19, 10:05 PM.

Details

Summary

Because the fake Switch User item in the switcher UI counts as a session, the button
was always being shown. But if and when that changes and the fake item is no longer shown,
the Switch User button itself will stops being shown potentially locking out
non-logged-in users in multi-user computers where the only logged-in user has locked the
screen.

This patch drops the conditional check entirely and always shows the Switch User
button when session creation/switching is possible.

Test Plan

No change; Switch User button is still shown.

Diff Detail

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ngraham created this revision.Mon, Aug 19, 10:05 PM
Restricted Application added a project: Plasma. · View Herald TranscriptMon, Aug 19, 10:05 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
ngraham requested review of this revision.Mon, Aug 19, 10:05 PM

With this patch how would someone create the first new session?

ngraham added a comment.EditedMon, Aug 19, 10:44 PM

With this patch how would someone create the first new session?

With Kickoff/kicker/dash/Krunner > Switch User, which goes directly to the session management screen.

The comment in the code makes it clear that the current behavior of showing the button is a bug. If we decide that we like the current behavior and we do want the button to be always shown, then currently it will be inappropriately invisible if sessionsModel.showNewSessionEntry is ever set to false (as I do in D23283), so something still needs to be fixed.

Seems to me that we have two options:

  • Land this patch so the Switch User button is only shown when there's already another session to switch to
  • Remove the visible: check entirely so the Switch User button is always shown
ngraham added a reviewer: VDG.Tue, Aug 20, 2:37 AM

Adding VDG for comment on the above ^^

filipf added a subscriber: filipf.Tue, Aug 20, 8:51 PM

What if someone else is logged in, they locked their screen and I want to switch the session?

Might be something urgent, and if I don't have a button to leave their lock screen I might have to reboot and lose their work.

No more Switch User button on the login screen unless there are multiple sessions

You mean lock screen?

ngraham edited the test plan for this revision. (Show Details)Tue, Aug 20, 9:03 PM

What if someone else is logged in, they locked their screen and I want to switch the session?

Then there are multiple sessions because you're already logged-in, so the button will be visible and you'll be able to get to your session. Or are you thinking with a multi-user system where user A logs in and locks the screen, and then user B wants to log in? Hmm.

What if someone else is logged in, they locked their screen and I want to switch the session?

Or are you thinking with a multi-user system where user A logs in and locks the screen, and then user B wants to log in? Hmm.

Yep, that's the scenario I had in mind.

ngraham updated this revision to Diff 64172.Tue, Aug 20, 9:25 PM

Always show the button

ngraham retitled this revision from [Lock screen] Fix bug causing "Switch User" button to always be visible to [Lock screen] Always show "Switch User" button regardless of whether fake "switch user" item is shown.Tue, Aug 20, 9:28 PM
ngraham edited the summary of this revision. (Show Details)
ngraham edited the test plan for this revision. (Show Details)
filipf accepted this revision.Tue, Aug 20, 9:42 PM

I don't know if there would be some use in keeping sessionsModel.canStartNewSession && sessionsModel.canSwitchUser around, but the button should always be visible (when technically possible I guess).

This revision is now accepted and ready to land.Tue, Aug 20, 9:42 PM

No, you're right, those should still be there.

ngraham edited the summary of this revision. (Show Details)Tue, Aug 20, 9:45 PM
ngraham updated this revision to Diff 64177.Tue, Aug 20, 9:45 PM

Re-add other conditions

ngraham edited the summary of this revision. (Show Details)Tue, Aug 20, 9:45 PM
This revision was automatically updated to reflect the committed changes.