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 758873 - bad choice of default behaviour reveals password in login screen
bad choice of default behaviour reveals password in login screen
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: login-screen
3.18.x
Other Linux
: Normal normal
: ---
Assigned To: Ray Strode [halfline]
gnome-shell-maint
: 786686 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2015-11-30 22:48 UTC by Ben
Modified: 2017-08-23 18:55 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
popupMenu: Don't handle key events for insensitive source (1.67 KB, patch)
2017-03-25 13:49 UTC, Florian Müllner
committed Details | Review

Description Ben 2015-11-30 22:48:22 UTC
On a slow computer it  is enough to  press enter three times after entering the password to reveal it (eg. because one is impatient?). Even on modern hardware this can happen with slightly misspelled passwords because of the 3 (?) seconds default sleep upon entering a wrong password.

This is due  to a bad choice of default actions. When the password is already entered another  press of the enter key opens the menu with  the "show password" option marked as default action, so pressing enter again in fact does reveal the password.

I am currently on 3.18.2 but this behavior is not new...
Comment 1 Michael Catanzaro 2016-01-11 23:47:16 UTC
When the sign-in button is pressed, we should disable the context menu.
Comment 2 matt.weiss 2017-03-25 00:38:42 UTC
I have been running into this recently (without knowing why my password is showing). It was pretty alarming. I would greatly appreciate if this behavior was changed.
Comment 3 Florian Müllner 2017-03-25 13:49:21 UTC
Created attachment 348699 [details] [review]
popupMenu: Don't handle key events for insensitive source

Generalizing menu toggling via keyboard in commit 1d58ea25ab6f047
fixed keynav in many places, but it turns out that it also adds
unexpected interactions in some cases where the source is not
button-like, as for example the entry context menus provided by
ShellEntry. Commit e33c68a415 fixed one case, however it is still
possible for plain enter/space to unexpectedly trigger the menu
if the entry itself doesn't consume the event, which is the case
when ClutterText:editable is false. However for a general fix, it
makes more sense to consider the source actor's :reactive property
and disable toggling menus via keyboard when they cannot be toggled
by pointer either - expecting non-editable entries to be non-reactive
as well seems like a reasonable assumption, and indeed all our code
follows that pattern.
Comment 4 Florian Müllner 2017-03-25 13:51:58 UTC
Another less-general fix would be to override the _onKeyPress() handler in EntryMenu with a no-op to make sure that the menu is only brought up via the ::popup-menu signal.
Comment 5 Ray Strode [halfline] 2017-03-27 14:46:49 UTC
Review of attachment 348699 [details] [review]:

This makes sense to me.  another idea would be to connect to notify::reactive on the source actor and call setSensitive or something like that.
Comment 6 Florian Müllner 2017-04-04 18:59:54 UTC
Attachment 348699 [details] pushed as 647c8df - popupMenu: Don't handle key events for insensitive source

(In reply to Ray Strode [halfline] from comment #5)
> This makes sense to me.  another idea would be to connect to
> notify::reactive on the source actor and call setSensitive or something like
> that.

This sounds like a good option for making the menu insensitive (although I suspect it will still pop up?), but setting it automatically to sensitive again when the source becomes reactive may interfere with explicitly set sensitivity, so I'm a bit wary here.
I'll go with my original patch ...
Comment 7 Ray Strode [halfline] 2017-08-23 18:55:20 UTC
*** Bug 786686 has been marked as a duplicate of this bug. ***