GNOME Bugzilla – Bug 331164
No facility to lock the screen on lid closure
Last modified: 2006-02-18 21:27:29 UTC
There is currently no facility in g-p-m to lock the screen when a laptop lid is closed. This is a very handy feature which we would like in ubuntu
It should lock if you have g-p-m configured to suspend on lid event and gnome-screensaver configured to lock the screen. It currently does not lock when it just turns off the screen using DPMS. Is this wrong?
What I meant was that there should be an option for "lock only" I am currently working on a patch for this for Ubuntu and will attach it once I'm done.
Cool, thanks -- there might be some refactoring work required for this, but then I'm sure you've seen the code.
Daniel, What's the use case of having a lock only? Is it for when we are connected to AC (we don't go to sleep in that case) If so I think we should not introduce an extra preference but just lock the screen if screenlocking is enabled in gnome-screensaver Jaap
My primary use case, and that of my boss, is conferences. Consider you are at a conference and you want to get from room A to room B without disconnecting the TCP/IP sessions you have running. You simply want to close the lid and walk with the laptop between the rooms (which might take 2 or 3 minutes, but never leaving wireless range) and when you get there, open the lid and carry on working. Now consider that while you trust the person you're sat next to to watch your laptop while you visit the bathroom, you don't trust them not to send silly messages from your IRC window so you close the lid to lock the screen. Essentially that's my primary use-case. So it's an action on lock-screen and really didn't take me much work to implement (although it may be a touch ugly) I'll work up a diff and attach it.
Yes, I think that's a valid use-case.
Created attachment 59401 [details] [review] debdiff between two ubuntu package versions I believe that the Makefile.in patch is incidental (certainly I didn't do it, I think autotools or some vile packaging tool did) and obviously you don't want the debian/changelog patch, but the rest is the basic implementation of a lock action.
I like the patch, but I've got some concerns about how "obvious" all this locking stuff is. I see three things we do: 1. lid key action 2. suspend key action 3. idle timeout action where we currently do no action, suspend, hibernate etc. Do we not want options such as * No action * Lock the screen * Suspend * Suspend and Lock * Hibernate * Hibernate and Lock If not, I think this needs to be made clearer. Maybe a checkbox next to each of the three options "Lock the screen" as sometimes we want to suspend and not lock, but sometimes we want to hibernate *and* lock depending on the event type. I know we currently use the setting in g-s for locking the screen (on hibernate and suspend), but I think user feedback has shown that people want a more fine grained control due to indervidual circumstances.
I think the no/lock/susp/susp+lock/hib/hib+lock sequence is nice. I went for minimal intervention when I did my patch but such a set of choices wouldn't be too hard to extend into place. Personally I don't see the user wanting "lock only" when they hit their suspend key, but not offering it would seem almost churlish when it costs us nothing to offer it. The checkbox idea is nice except then "lock only" would be "Do nothing, and lock" which would confuse people I feel. I believe it'd be rare to want suspend/hibernate and not lock, but I guess it's possible some people wouldn't want it, and again if it's not expensive to implement it makes sense to offer it. My patch was the minimum required to fulfil the requirements of an Ubuntu user, I'd love to see a more comprehensive patch come in from upstream.
>Maybe a checkbox next to each of the three options I just asked my non-technical girlfriend (a great source of the "best way" to do things) for what would be the best way to select this option. She said a checkbox would be the most intuitive option as it keeps things simple. >I believe it'd be rare to want suspend/hibernate and not lock, but I guess it's possible some people wouldn't want it, and again if it's not expensive to implement it makes sense to offer it. Her comment was "I wouldn't want to lock it" :-) Richard.
non-techie people are a good source of ideas. Okay, so a checkbox would be best for her, in that case we need a better text than "No action" because they would look bizarre. How about: * Do not suspend * Suspend to RAM * Suspend to Disk With the tickybox being "Lock screen"
Yes, I agree about the "No action". Suspend and Hibernate seem a better choice of words as they are not getting technical. I know s-t-d and s-t-r is what they actally do, but I don't think a new user should have to comprehend this. Although my girlfriend seems to be okay with Suspend to RAM, even if it did have to be explained to her. :-) If you leave the actual patch for adding a checkbox to me, I'll do some other tidy ups too. Richard.
I'm not sure what the 'no action' text could be if we keep suspend vs hibernate, but I'm sure you'll come up with something. Thanks for being so positive about this, I look forward to the patch.
What about abstracting it further to a combobox: |----- LOCK OPTIONS ---| 0. Never lock the screen 1. Always lock the screen 2. Lock after hibernate 3. Lock after suspend I think this covers all use-cases... Because we also need to think about lock policy if the user invokes the Suspend DBUS method, and I don't want to add a bazillion [1] checkboxes. Richard. [1] usually approximatly equal 4.
Guys, I really don't like adding more options to gpm. Especially since the case described is not really common and I think can be solved easily by doing the following, which does not add any check or comboboxes What about the following. 1. We always put the screen to sleep when the lid is closed 2. We offer a choice, between "Do nothing", "Hibernate" and "Suspend" 3. We lock the screen if the lid is closed if screen locking is enabled in gnome-screensaver. This way people get automatically what they want. If they want privacy they enable locking in gnome-screensaver. If they don't want it they just disable it. Really simple Jaap
Daniel, is this acceptable?
I don't think Jaap's idea is quite complex enough. I know that GNOME strives for simplicity and obviousness but this is a situation where personal taste is something we can't "correct" in any easy way. Not least of which, my non-techie (father) prefers his screensaver not to auto-lock but he does want his laptop to lock when he closes the lid. Currently he has to go through the menu to lock the screen before closing his laptop and he finds that a bad regression from when the acpi scripts locked his screen for him automatically. The lock combo seems more sensible to me, so long as it defaults to the more secure option of "always lock" I certainly prefer the lock combo box to a tickybox next to each action option.
in light of the resolution of bug 329512 which came as part of 2.13.90 I am changing how the Ubuntu package does this to simply always locking the screen if possible (if permitted by g-s-s) when the lid is closed, regardless of ac or anything else. This behaviour is the best we can come up with pending your decision about how we go about sorting the prefs out.
Created attachment 59517 [details] screenshot of possibilities Which of these options looks the easiest to a user? I prefer the 4 checkboxes at the bottom. I agree to a degree with Daniel, *and* with Jaap with regard to options, but I do think we need to give the users what they want, rather than restrict the options to a few common use-cases. But then you can go too much the other way as I think KDE has done (my opinion only) where it's just crazy. I would appreciate both your views on the screenshot, thanks.
Option 1 combobox only let's you select one option which is not good in this case Option 2 Too many options. checkbox 4 "When DBUS event is received" should definitly not be there. Who of the normal users knows what that is? Also "when the computer is left idle" should not be there because that's already specified in gnome screensaver. Also when we are on AC and close the lid we don't want to have screenlocking otherwise we have to open bug 329512 again. Also do I have screenlocking when I select suspend or hibernate from the panel menu?? Conclusion I think neither of the options is really good. To make everybody happy (simple GUI, and highly configurable for the very demanding users among us) I think the best is to have good default options and not displaying them in the GUI. The demnanding people can then change gconf keys. So then I think we would need the following gconf keys gconf key default screen_lock_on_hibernate TRUE screen_lock_on_suspend TRUE screen_lock_on_lid_battery TRUE screen_lock_on_lid_on_ac FALSE
KDE is definitely down the "OMG which button do I push now?!?!" route, which is not what we want to see. I don't think people would be surprised by the screen locking on hibernate or suspend, so we leave that behaviour like it is. The difficulty in constructing a UI comes from the resolution to bug 329512 which meant that the lid action didn't happen on AC. It seems to me that one possible solution which would make life easier again is to alter the list of actions to include "lock" again (as per my original idea) and then to offer a tickybox (defaulting to unticked) which is "Also apply lid action when on AC power" Jaap: What would you think of that?
Daniel, can you comment on bug: http://bugzilla.gnome.org/show_bug.cgi?id=331132 As I think this might be acceptable also. Richard.
That looks pretty good to me. (comments are on that bug as you know)
2006-02-18 Richard Hughes <richard@hughsie.com> * data/gnome-power-manager.schemas.in, src/gpm-prefs.h: Add lock_ac_lid, lock_battery_lid, lock_button_suspend, and lock_sleep_idle. * data/gpm-prefs.glade: Enable the new lock checkboxes. * src/gpm-prefs.c (gpm_prefs_checkbox_lock_cb, gpm_prefs_setup_checkbox): Add infrastructure so checkboxes can be tied to gconf keys. (setup_power_buttons, setup_sleep_type, setup_ac_actions, setup_battery_actions): Add the checkboxes into these handlers. NOTE, these keys get set correctly, but are not handled by the manager yet. Jaap, I've exposed all these as checkboxes for now, we can always hide them if it's required later. For testing, and to get feedback, they are in the UI for now.
2006-02-18 Richard Hughes <richard@hughsie.com> * src/gpm-manager.c (manager_policy_do): Add a do_lock argument to this function so we can now specificy the lock policy according to the event that caused the policy to be performed. Do the locking here rather than gpm_manager_suspend and gpm_manager_hibernate. * src/gpm-manager.c (idle_changed_cb): BUGFIX: Do the idle action -not- the critical action. Not sure how nobody has triggered this yet. * src/gpm-manager.c (suspend_button_pressed, lid_button_pressed): Query and use the new lock gconf arguments. We probably need to add more arguments for stuff like DBUS calls and tray clicks, but I don't think these have to be in the UI.
Right, I know the UI looks a bit rubbish, but is everyone happy with the options? I'm playing with the idea of a master checkbox "Use simple lock settings" which just do the gnome-screensaver defaults (like we had before) and that removes all the "also lock the screen" checkboxes. Good idea/bad idea?
(In reply to comment #26) > Right, I know the UI looks a bit rubbish, but is everyone happy with the > options? I like the addition of "Blank screen" option. It's much better than "do nothing" > > I'm playing with the idea of a master checkbox "Use simple lock settings" which > just do the gnome-screensaver defaults (like we had before) and that removes > all the "also lock the screen" checkboxes. Good idea/bad idea? After giving this all some more thought I really don't like having the lock settings in the GUI. 1. Why are they there for closing the lid and not for hibernation and suspending? 2. I think we're seperating to many cases now for locking. We're looking if we are on ac or battery or if we push a button or if we were idle. lock_ac_lid lock_battery_lid lock_button_suspend lock_sleep_idle Furthermore it's something that a lot of people probably never will change. IMHO there should be just 2 gconf settings concerning locking lock_on_suspend lock_on_hibernate both set to TRUE by default. If lid closure is set to "blank screen" we just don't lock the screen. People wanting to lock it can do so by just locking the screen (by using a shortcut key or choosing lock screen from the menu) before closing the lid 3. I agree the check boxes in GUI are making the dialog ugly at the moment 4. The case daniel is describing is uncommon. I don't see the problem that his boss first has to lock the screen before closing the lid. He probably already needed to go to g-p-p first to change his setting for lid-closure anyway. So what's the problem of hitting the screen lock button.
Jaap, case 4 is *NOT* uncommon. Consider that the company I work for has approximately fifty employees, almost all of which want the lock-on-lid-close behaviour. Everyone I've queried wants it apart from those who want it to suspend on lid-close (and of course lock also) If we had all the lock options in gconf, but hide the UI by default then I think that's best. Perhaps have a conf key to show the UI. Then power users can turn it on easily and configure it how they want. While I think GNOME's reduction of options is honourable, I also think that it is a terrible idea to not include functionality which is very common in the outside world. Consider the toshiba power manager in windows if you ever want to see something which has more options than you can shake a stick at while being easy enough to use that my *mother* can configure her laptop how she wants.
>While I think GNOME's reduction of options is honourable, I also think that >it is a terrible idea to not include functionality which is very common in >the outside world. Agreed, see my points: http://bugzilla.gnome.org/show_bug.cgi?id=331448#c11 And the toshiba power-saver was the first thing to get removed on my windows xp partition. You have *no idea* how horrible the UI was. :-) >If we had all the lock options in gconf, but hide the UI by default I'm playing with this now. Richard.
Created attachment 59663 [details] example screenshots Guys, what about something like this?
(In reply to comment #30) > Created an attachment (id=59663) [edit] > example screenshots > > Guys, what about something like this? > This is complete crack if you ask me. Because of the weird combinations of locking and states it get's very confusing. How does somebody know what "use default locking rules" means? With this dialog I also cannot know what happens if I for example hibernate from the panel menu. (In reply to comment #28) > Jaap, case 4 is *NOT* uncommon. Consider that the company I work for has > approximately fifty employees, almost all of which want the lock-on-lid-close > behaviour. Everyone I've queried wants it apart from those who want it to > suspend on lid-close (and of course lock also) > I guess of this fifty people a large majority will have it set to suspend. But if we really have to have the option. Then I would say no UI just gconf settings. gconf settings then would be default lock_on_suspend TRUE lock_on_hibernate TRUE lock_on_close_lid TRUE
Sorry I meant lock_on_close_lid FALSE Power users who have no suspending on lid_close or system admins can then always set it to TRUE
Okay, should I get rid of the locking checkboxes, and just have gconf preferences for people that want to turn the locking on/off? Sane defaults, and then give power users the option to tweak stuff in gconf? Richard.
Reading bug 331448 gave me even some more thoughts. What we also could do ofcourse which does not take any prefs is to look at the setting of gnome-screensaver. People who have locking enabled over there (people caring about privacy) get also locking with all actions of gnome-power-manager. People not caring about privacy will never get an unlock dialog. This should make everybody happy I think. * It's simple * It's logical * It's present in a UI * It's a gconf setting so admin can force it to a certain setting * People caring about privacy get a lock dialog everywhere * People not caring about privacy do not get a lock dialog What do you think about this?
Something in between of course is to have a gnome_screensaver_locking TRUE if that is set to FALSE the other options are taken into account.
> gnome_screensaver_locking = TRUE This would keep power-users happy, and also provide an easy to explain "if it's set to lock in g-s then g-p-m locks" for the new user. And we can just document how to change this setting to keep everyone happy. This can also be set by sabayon for an enterprise if required. Daniel, is that okay with you?
Yes, that would be perfect.
Oh, can we make sure it's in the yelp too?
2006-02-18 Richard Hughes <richard@hughsie.com> * data/gpm-prefs.glade, src/gpm-prefs.c (gpm_prefs_setup_checkbox, gpm_prefs_checkbox_lock_cb): Remove the lock checkboxes. :-) * data/gnome-power-manager.schemas.in, src/gpm-prefs.h: Add lock_use_screensaver_settings to *finally* fix bug (#331164) * src/gpm-manager.c (manager_policy_do): Allow over-ride with settings from gnome-screensaver. Default to using gnome-screensaver settings as it's most obvious. Sabayon can be used to tweak these settings per domain if required. I think this keeps everyone happy. I'm closing this bug as I know the number off by heart now. Daniel, can you create a bugzilla for adding this information to the yelp file and we'll discuss the best way to do that in the new bug.