Created attachment 213793 [details] [review]
userMenu: Fix 'Lock' lockdown setting

Oops, embarrassing typo ...

Created attachment 213794 [details] [review]
userMenu: Hide "Log out" item on single user machines

Ignoring remote logins, the "Log out" action is meaningless for
single user setups. Do not show it in that case.

Created attachment 213795 [details] [review]
userMenu: Update to latest mockups

Technically those are minor changes, but people will love us anyway:

  - do not hide "Power Off" behind the alt-switch
  - remove "Online Accounts" item
  - regroup the remaining items

Review of attachment 213793 [details] [review]:

Hah. How long has this been here?

Comment on attachment 213793 [details] [review]
userMenu: Fix 'Lock' lockdown setting

Attachment 213793 [details] pushed as 83d3225 - userMenu: Fix 'Lock' lockdown setting

Not too long apparently - it was introduced when changing show()/hide() to visible = ...

Whoops...

Review of attachment 213794 [details] [review]:

Uh, sure. This does mean that the recommended way of shutting down your computer is no longer going to gdm, right?

::: js/ui/userMenu.js
@@ +550,3 @@
     _updateSwitchUser: function() {
         let allowSwitch = !this._lockdownSettings.get_boolean(DISABLE_USER_SWITCH_KEY);
         if (allowSwitch &&

It seems I forgot one show/hide path in updateSwitchUser. Can't win 'em all, I guess. Fix that one while you're at it, please.

Review of attachment 213795 [details] [review]:

Aren't you missing the "Power Off switches to Suspend on alt modifier"?

Comment on attachment 213794 [details] [review]
userMenu: Hide "Log out" item on single user machines

Attachment 213794 [details] pushed as 518282e - userMenu: Hide "Log out" item on single user machines


> Uh, sure. This does mean that the recommended way of shutting down your
> computer is no longer going to gdm, right?

We are going to "unhide" Power Off, so yes.


> It seems I forgot one show/hide path in updateSwitchUser. Can't win 'em all, I
> guess. Fix that one while you're at it, please.

Sure.

Created attachment 213809 [details] [review]
userMenu: Update to latest mockups

(In reply to comment #8)
> Aren't you missing the "Power Off switches to Suspend on alt modifier"?

Indeed I did, thanks!

Review of attachment 213809 [details] [review]:

Looks fine to me.

(Why the removal of the "Online Accounts" item, though? I also don't understand the grouping of Power Off and System Settings, but whatever)

Attachment 213809 [details] pushed as afaa5c2 - userMenu: Update to latest mockups

Leaving the bug open for now for the "Switch Session" item.


(In reply to comment #11)
> (Why the removal of the "Online Accounts" item, though?

My guess is that it's not too useful once you have set up your accounts. And considering that it appears in the "first-run" mockups, chances are that users won't ever need the item at all, so just having it in System Settings is enough. But well, just guessing ;-)


> I also don't understand the grouping of Power Off and System Settings,
> but whatever)

"whatever" sums it up pretty good for me as well :)

Does it mean that from now on I'll be unable to logout without using ctrl-alt-backspace/altgr+sysrq+k/etc.? If it's true, I'm certainly against this decision. Since I, and I think other people too, have my own reasons for having that option right there even on a single user system. One of them is the ability to change sessions, e.g. logout from Gnome and login back, say, in Unity or whatever instead of Gnome.

(In reply to comment #13)
> Does it mean that from now on I'll be unable to logout without using
> ctrl-alt-backspace/altgr+sysrq+k/etc.? [...] Since I have my own reasons 
> for having that option right there even on a single user system. One of 
> them is the ability to change sessions

Please read the design document linked in the bug description - "Single-user, multiple sessions" systems are explicitly mentioned there. It is not currently implemented (see the question for designers above), but will be there in time for 3.6.

Created attachment 213959 [details] [review]
userMenu: Offer "Switch Session" item on multi-session systems

Since commit 518282e169, we hide both "Switch User" and "Log out"
on single-user systems. However, if there is more than one session
available, users may still want to get back to the login manager
to change sessions.
Add both "Log out" and "Switch Session" items in that case.


This implements the "Switch Session" switch, assuming that multi-user > multi-session (e.g. on multi-user, multi-session systems, "Switch User" is shown)

Review of attachment 213959 [details] [review]:

I'm not sure how I like depending on GdmGreeter from the non-gdm code. I also don't think we require libgdmgreeter anywhere -- we'd have to change configure.ac, right?

(In reply to comment #16)
> I'm not sure how I like depending on GdmGreeter from the non-gdm code.

The only alternative I see is reimplementing the method ourselves, e.g. scanning for .desktop files exactly where gdm is scanning as well - but really, that sounds like GdmUser all over again ...


> I also don't think we require libgdmgreeter anywhere -- we'd have to change
> configure.ac, right?

No, the dependency on the typelib is enough (otherwise "gdm mode" wouldn't work at all, after all it's not a different executable).

(In reply to comment #17)
> The only alternative I see is reimplementing the method ourselves, e.g.
> scanning for .desktop files exactly where gdm is scanning as well - but really,
> that sounds like GdmUser all over again ...

Doesn't gnome-session have a library that we can use?

*** Bug 673199 has been marked as a duplicate of this bug. ***

(In reply to comment #18)

> Doesn't gnome-session have a library that we can use?

No

Review of attachment 213959 [details] [review]:

Go for it, then.

Please keep the "Log out" entry for single user systems.
Even in a single user system it is useful if you can relogin because you want to restart your session or you want to start another DE/window manager.

Uh, yes, this patch adds back the Log Out entry for multi-session systems.

Attachment 213959 [details] pushed as ca26347 - userMenu: Offer "Switch Session" item on multi-session systems

(In reply to comment #22)
> Even in a single user system it is useful if you can relogin because you want
> to restart your session or you want to start another DE/window manager.

This adds back "Log Out"/"Switch User/Session" for the "start another DE/window manager" case. For other reasons (change language, update, ...) see bug 657011.

I'm curious if we should show Suspend by default on desktops, given that you can't easily close the lid.