After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 686740 - difficult to transition to the new lock screen
difficult to transition to the new lock screen
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
: 688203 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2012-10-23 22:41 UTC by William Jon McCann
Modified: 2013-03-23 16:57 UTC
See Also:
GNOME target: 3.8
GNOME version: ---


Attachments
screenShield: Remove bump on key press (2.93 KB, patch)
2012-12-08 08:03 UTC, Jasper St. Pierre (not reading bugmail)
needs-work Details | Review
screenShield: Remove bump on key press (3.09 KB, patch)
2013-03-05 07:06 UTC, Jasper St. Pierre (not reading bugmail)
needs-work Details | Review
screenShield: Don't wait until the dialog is loaded before opening it (3.55 KB, patch)
2013-03-05 07:06 UTC, Jasper St. Pierre (not reading bugmail)
accepted-commit_now Details | Review
screenShield: Forward key presses to tne entry when raising the shield (1.66 KB, patch)
2013-03-05 07:07 UTC, Jasper St. Pierre (not reading bugmail)
reviewed Details | Review
screenShield: Move opening of screen shield to key press (1.64 KB, patch)
2013-03-11 18:37 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
screenShield: Remove bump on key press (3.09 KB, patch)
2013-03-11 18:37 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
screenShield: Don't wait until the dialog is loaded before opening it (3.55 KB, patch)
2013-03-11 18:37 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
screenShield: Forward key presses to tne entry when raising the shield (1.65 KB, patch)
2013-03-11 18:37 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
0001-screenShield-Also-unlock-on-Return-Enter.patch (1.05 KB, patch)
2013-03-12 16:58 UTC, Colin Walters
committed Details | Review

Description William Jon McCann 2012-10-23 22:41:36 UTC
Many users of gnome-screensaver / gnome 2 are accustomed to being able to just type their password without looking when unlocking the screen. We don't allow this now with the new shield. Typing any characters just "bumps" up the shield a little bit. One user I observed today thought this was offensive - like it was mocking him or taunting him.

In my original conception of the lock shield I wanted typing to lift the shield. The reason for this is compatibility with existing practice and the fact that we don't need to guard the screen from stray touches when the input is purely keyboard based.

He also had no idea that escape lifted the shield at all.
Comment 1 Jakub Steiner 2012-10-23 22:59:31 UTC
My original thought of just wanting to get to the notifications/music controls on a display in sleep by hitting a random key is easily worked around by using the Esc key as it toggles the shield on and off.

I've heard this complaint a number of times and there might be something about that trolling bit :)
Comment 2 Rui Matos 2012-10-24 00:37:57 UTC
I agree with this. It's how the Windows 8 lock shield behaves FWIW. The timing is also important to reduce frustration IMO, see bug 686745.
Comment 3 Allan Day 2012-10-31 19:16:18 UTC
Me, Jon and Jakub discussed this today. We agreed that the curtain should be raised when any key is pressed, with the exception of the shift keys.

There was some discussion about not raising for the return key, also, but I have to confess that I'm not 100% on that one.

It would also be good to not raise the curtain if the monitor is switched off. In this case, keyboard input would simply wake up the display.
Comment 4 Jasper St. Pierre (not reading bugmail) 2012-10-31 19:27:02 UTC
(In reply to comment #3)
> Me, Jon and Jakub discussed this today. We agreed that the curtain should be
> raised when any key is pressed, with the exception of the shift keys.

Would the key you pressed to raise the lock screen also enter something into the password field?
Comment 5 Allan Day 2012-10-31 19:45:56 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > Me, Jon and Jakub discussed this today. We agreed that the curtain should be
> > raised when any key is pressed, with the exception of the shift keys.
> 
> Would the key you pressed to raise the lock screen also enter something into
> the password field?

Good question. The password field should only accept entry once it is visible.
Comment 6 William Jon McCann 2012-10-31 20:07:34 UTC
What we used to do in gnome-screensaver was save all the keystrokes in a buffer and forward them to the text entry once it became visible. I think think of a reason why we wouldn't want that. It ensures that those keys don't get sent to open windows in the case where the screen isn't locked too.

Maybe Ray has some thoughts.
Comment 7 Florian Müllner 2012-10-31 20:12:06 UTC
(In reply to comment #6)
> What we used to do in gnome-screensaver was save all the keystrokes in a buffer
> and forward them to the text entry once it became visible.

Anything else doesn't make sense to me to be honest, in particular as passwords are masked by default (e.g. if input is dismissed until the entry becomes visible, users would need to count bullets or something to figure out what's in the entry)
Comment 8 Guillaume Desmottes 2012-11-07 15:00:51 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > Me, Jon and Jakub discussed this today. We agreed that the curtain should be
> > raised when any key is pressed, with the exception of the shift keys.
> 
> Would the key you pressed to raise the lock screen also enter something into
> the password field?

Definitely. User should just has to enter his password and Enter to unlock, no need to type any extra key.
Comment 9 Matthias Clasen 2012-11-13 00:08:13 UTC
*** Bug 688203 has been marked as a duplicate of this bug. ***
Comment 10 Ray Strode [halfline] 2012-11-13 16:44:21 UTC
> Maybe Ray has some thoughts.

A couple:

- with gnome-screensaver people would generally walk up to their machines and do one of:

 1) hit escape, then type their password
 2) hit space, then type their password
 3) hit enter, then type their password
 4) just start typing their password right away

We definitely used to queue up what they were typing, and replay it at the appropriate time. We also filtered out the initial space, escape, or enter key when doing the replay, because of the first three cases above.  Lastly, we filtered out Tab, and arrow keys with this comment:

/* Don't queue keys that may cause focus navigation in the dialog */

I think if gnome-shell mimiced the gnome-screensaver behavior, it would be more convenient than what it currently does.

- I do think it's important that media keys and the like don't raise the shield especially if there is an onscreen media control on the shield

- If there is something "focusable" on the screen and someone tries to navigate focus in it (say with alt-tab, tab, arrow keys, whatever), we shouldn't raise the shield but instead move focus to that, or move the focus around in that.
Comment 11 Jasper St. Pierre (not reading bugmail) 2012-12-08 08:03:23 UTC
Created attachment 231025 [details] [review]
screenShield: Remove bump on key press

Any key press will now raise the shell. Note that the key press will
not be forwarded to the entry yet.
Comment 12 Allan Day 2013-01-16 10:13:19 UTC
Everything in comment 10 sounds right to me. I guess that means that this still needs work.
Comment 13 Allan Day 2013-02-28 10:23:10 UTC
I thought this was fixed, but it seems that the patch hasn't been committed. Can we get this sorted for 3.8?
Comment 14 Florian Müllner 2013-02-28 11:17:01 UTC
Review of attachment 231025 [details] [review]:

(In reply to comment #13)
> I thought this was fixed, but it seems that the patch hasn't been committed.
> Can we get this sorted for 3.8?

I'd like to see that, but in my opinion we need forwarding to the entry - with this patch, starting to type will appear to do the right thing (raise the shield and start the password conversation), but only the former is true, so users will end up with an authentication failure unless they learn to type the first letter twice.
Comment 15 Jasper St. Pierre (not reading bugmail) 2013-02-28 14:48:03 UTC
The reason was because of IBus support, but it's been acknowledged that we don't want that in the password entry. I'll create and attach an updated patch soon.
Comment 16 Cosimo Cecchi 2013-03-01 17:51:55 UTC
Note that now with bug 643111 fixed media keys also bump the screen shield in addition to their action.
Comment 17 Jasper St. Pierre (not reading bugmail) 2013-03-05 07:06:27 UTC
Created attachment 238091 [details] [review]
screenShield: Remove bump on key press

Any key press of a character-emitting key will now raise the shell.
Note that the key press will not be forwarded to the entry yet.



Rebased on master.
Comment 18 Jasper St. Pierre (not reading bugmail) 2013-03-05 07:06:33 UTC
Created attachment 238092 [details] [review]
screenShield: Don't wait until the dialog is loaded before opening it

If we wait asynchronously, key presses while the shield is opening
will be dropped in the void.
Comment 19 Jasper St. Pierre (not reading bugmail) 2013-03-05 07:07:02 UTC
Created attachment 238094 [details] [review]
screenShield: Forward key presses to tne entry when raising the shield

Attaching this for now. There still seem to be some key presses
that are lost somewhere. I don't know why. Hopefully gcampax or
someone else can investigate why.
Comment 20 Giovanni Campagna 2013-03-11 16:38:59 UTC
Review of attachment 238091 [details] [review]:

::: js/ui/screenShield.js
@@ +603,2 @@
+        this._ensureUnlockDialog(true, true);
+        this._hideLockScreen(true, 0);

You should use _liftShield here, to account for key presses while not locked.
Comment 21 Giovanni Campagna 2013-03-11 16:40:25 UTC
Review of attachment 238092 [details] [review]:

Ugh... I think originally loaded had more sense, but yeah, this is definitely correct now.
Comment 22 Giovanni Campagna 2013-03-11 16:41:29 UTC
Review of attachment 238094 [details] [review]:

::: js/ui/screenShield.js
@@ +603,3 @@
         this._ensureUnlockDialog(true, true);
+
+        if (!GLib.unichar_isspace(unichar))

Shouldn't you check that unichar is valid here (ie is not esc)?
Comment 23 Jasper St. Pierre (not reading bugmail) 2013-03-11 18:37:28 UTC
Created attachment 238612 [details] [review]
screenShield: Move opening of screen shield to key press

This makes the screen shield much more responsive.
Comment 24 Jasper St. Pierre (not reading bugmail) 2013-03-11 18:37:32 UTC
Created attachment 238613 [details] [review]
screenShield: Remove bump on key press

Any key press of a character-emitting key will now raise the shell.
Note that the key press will not be forwarded to the entry yet.
Comment 25 Jasper St. Pierre (not reading bugmail) 2013-03-11 18:37:35 UTC
Created attachment 238614 [details] [review]
screenShield: Don't wait until the dialog is loaded before opening it

If we wait asynchronously, key presses while the shield is opening
will be dropped in the void.
Comment 26 Jasper St. Pierre (not reading bugmail) 2013-03-11 18:37:39 UTC
Created attachment 238615 [details] [review]
screenShield: Forward key presses to tne entry when raising the shield
Comment 27 Giovanni Campagna 2013-03-11 18:55:35 UTC
Review of attachment 238612 [details] [review]:

Yes.
Comment 28 Giovanni Campagna 2013-03-11 18:55:55 UTC
Review of attachment 238613 [details] [review]:

Yes.
Comment 29 Giovanni Campagna 2013-03-11 18:56:12 UTC
Review of attachment 238614 [details] [review]:

Yes.
Comment 30 Giovanni Campagna 2013-03-11 18:57:48 UTC
Review of attachment 238615 [details] [review]:

And yes.

For the record, the stack was tested and even typing as fast as I can, I can go from black to unlocked without losing characters.

::: js/ui/screenShield.js
@@ +603,3 @@
         this._ensureUnlockDialog(true, true);
+
+        if (GLib.unichar_isgraph(unichar))

Nice!
Comment 31 Jasper St. Pierre (not reading bugmail) 2013-03-11 19:08:40 UTC
Attachment 238612 [details] pushed as dde20f0 - screenShield: Move opening of screen shield to key press
Attachment 238613 [details] pushed as 67615a0 - screenShield: Remove bump on key press
Attachment 238614 [details] pushed as 127f10e - screenShield: Don't wait until the dialog is loaded before opening it
Attachment 238615 [details] pushed as 209014b - screenShield: Forward key presses to tne entry when raising the shield


Same here, so closing.
Comment 32 Colin Walters 2013-03-11 19:33:33 UTC
Yep, looks good here.  Thanks a lot for fixing this one!
Comment 33 Colin Walters 2013-03-12 16:58:07 UTC
Created attachment 238713 [details] [review]
0001-screenShield-Also-unlock-on-Return-Enter.patch
Comment 34 Jasper St. Pierre (not reading bugmail) 2013-03-12 17:02:01 UTC
Review of attachment 238713 [details] [review]:

Just merge it with the if statement. ACN after that.
Comment 35 Paolo Bonzini 2013-03-23 06:18:25 UTC
With:

glib2-2.35.8
gobject-introspection-1.35.8
gjs-1.35.8
gnome-shell-3.7.92

I get the following error in the lock screen:

    JS ERROR: !!!   Exception was: Error: Expected type gunichar for Argument 'c' but got type 'number' (nil)
    JS ERROR: !!!     message = '"Expected type gunichar for Argument 'c' but got type 'number' (nil)"'
    JS ERROR: !!!     fileName = '"/usr/share/gnome-shell/js/ui/screenShield.js"'
    JS ERROR: !!!     lineNumber = '606'
    JS ERROR: !!!     stack = '"([object GObject_Object],[object GObject_Union])@/usr/share/gnome-shell/js/ui/screenShield.js:606
wrapper([object GObject_Object],[object GObject_Union])@/usr/share/gjs-1.0/lang.js:213

where line 606 has the call "GLib.unichar_isprint(unichar)".
Comment 36 Florian Müllner 2013-03-23 07:37:05 UTC
What clutter version? Please don't tell me we have code depending on unreleased clutter three days before final ...
Comment 37 Bastien Nocera 2013-03-23 08:14:53 UTC
You need clutter 1.3.10 for this.
Comment 38 Paolo Bonzini 2013-03-23 14:11:42 UTC
Not sure if it's related to my report, but I have clutter 1.13.8
Comment 39 Jasper St. Pierre (not reading bugmail) 2013-03-23 16:39:30 UTC
Bastien misspoke: he meant to say Clutter version 1.13.10.
Comment 40 Bastien Nocera 2013-03-23 16:57:22 UTC
(In reply to comment #39)
> Bastien misspoke: he meant to say Clutter version 1.13.10.

Completely typoed, thanks for the correction.