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 687112 - Login button should be insensitive until text has been entered in the password field
Login button should be insensitive until text has been entered in the passwor...
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: login-screen
3.7.x
Other Linux
: Normal normal
: ---
Assigned To: Ray Strode [halfline]
gnome-shell-maint
: 687653 (view as bug list)
Depends on:
Blocks: 685307
 
 
Reported: 2012-10-29 12:50 UTC by Allan Day
Modified: 2012-11-05 19:33 UTC
See Also:
GNOME target: ---
GNOME version: 3.7/3.8


Attachments
Patch (1.71 KB, patch)
2012-10-29 16:53 UTC, Stéphane Démurget
accepted-commit_now Details | Review
Patch for both the unlock and login dialogs (4.64 KB, patch)
2012-11-01 15:16 UTC, Stéphane Démurget
reviewed Details | Review
Patch without tabs (4.65 KB, patch)
2012-11-01 15:20 UTC, Stéphane Démurget
committed Details | Review
Revert "panel: programmatic anim. control of AnimatedIcon" (2.84 KB, patch)
2012-11-05 19:32 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review

Description Allan Day 2012-10-29 12:50:15 UTC
You can't login until something has been entered in the password field. We should therefore make the login button insensitive until you have entered some text:

https://dl.dropbox.com/u/5031519/gnome-shell/login/1-incomplete.png
Comment 1 Stéphane Démurget 2012-10-29 16:53:51 UTC
Created attachment 227561 [details] [review]
Patch
Comment 2 Allan Day 2012-10-30 12:47:24 UTC
Looks good to me. Just needs code review by someone who understands code. :)
Comment 3 Giovanni Campagna 2012-10-30 23:07:14 UTC
Review of attachment 227561 [details] [review]:

Yes, this looks fine, but before we declare this fixed, we need a similar patch for the login dialog too.
Comment 4 Stéphane Démurget 2012-11-01 15:16:11 UTC
Created attachment 227808 [details] [review]
Patch for both the unlock and login dialogs

Oops, I totally missed that. Here's a patch with the required changes on both dialogs.
Comment 5 Jasper St. Pierre (not reading bugmail) 2012-11-01 15:18:45 UTC
Review of attachment 227808 [details] [review]:

This seems to have some indentation issues.
Comment 6 Stéphane Démurget 2012-11-01 15:20:57 UTC
Created attachment 227809 [details] [review]
Patch without tabs

Sorry, I just saw there was tab vs space issues.
Comment 7 Giovanni Campagna 2012-11-04 01:55:50 UTC
Review of attachment 227809 [details] [review]:

Tested, looks fine and works fine.
It is ok to commit as is if you want, or you can merge it, but at some point LoginDialog needs the same sensitivity tracking that UnlockDialog does, i.e. the login button should be insensitive while verification is in process (after answerQuery and before the next _askQuestion)
Comment 8 Stéphane Démurget 2012-11-04 08:57:25 UTC
Giovanni: that's exactly what I was about to work on today :)

I let it rot a bit as I wanted to finish 657315 before, so I could wait for designers input between my tasks.
Comment 9 Stéphane Démurget 2012-11-04 09:00:36 UTC
Giovanni: I meant you can commit this one for me, I'm working on 687113, which is the other part of the sensitive work.

Allan split the bugs to simplify sharing, but the entry sensitivity should have been part of this bug from the start.
Comment 10 Jasper St. Pierre (not reading bugmail) 2012-11-04 14:21:54 UTC
Given that the login dialog and unlock dialog use the same UI, it probably makes sense to make it into a shared widget that does some part of the PAM step, and has a signal for handoff (raise modal, defer to gdm)
Comment 11 Stéphane Démurget 2012-11-04 19:36:11 UTC
That would be good to commit this patch before nonetheless, to avoid an additional rebase :)
Comment 12 Jasper St. Pierre (not reading bugmail) 2012-11-04 19:39:49 UTC
Yeah, it was just an off remark.
Comment 13 Allan Day 2012-11-05 10:21:26 UTC
Awesome work once again, Stéphane.

It would be cool if someone could file the bug about merging the login and unlock dialogs.
Comment 14 Giovanni Campagna 2012-11-05 17:14:55 UTC
(In reply to comment #10)
> Given that the login dialog and unlock dialog use the same UI, it probably
> makes sense to make it into a shared widget that does some part of the PAM
> step, and has a signal for handoff (raise modal, defer to gdm)

I might disagree with that. Yes, they look similar, but they are really very different in the implementation and actor hierarchy, so to unify them I'm afraid you'll get a generic monster with no actual benefit to the two codebases.
Comment 15 Jasper St. Pierre (not reading bugmail) 2012-11-05 19:04:53 UTC
(In reply to comment #14)
> I might disagree with that. Yes, they look similar, but they are really very
> different in the implementation and actor hierarchy, so to unify them I'm
> afraid you'll get a generic monster with no actual benefit to the two
> codebases.

Well yes, they're different in the implementation and actor hierarchy. My solution is to remove that.
Comment 16 Jasper St. Pierre (not reading bugmail) 2012-11-05 19:32:04 UTC
Created attachment 228182 [details] [review]
Revert "panel: programmatic anim. control of AnimatedIcon"

This reverts commit e04a4c39231ea1418591446d9b98aad4d7bcd2de.

This commit exposed an already-existing race condition in the panel
animation code that caused the shell to crash for some people.
Comment 17 Jasper St. Pierre (not reading bugmail) 2012-11-05 19:32:17 UTC
Comment on attachment 228182 [details] [review]
Revert "panel: programmatic anim. control of AnimatedIcon"

Attachment 228182 [details] pushed as d88002c - Revert "panel: programmatic anim. control of AnimatedIcon"
Comment 18 Jasper St. Pierre (not reading bugmail) 2012-11-05 19:33:18 UTC
*** Bug 687653 has been marked as a duplicate of this bug. ***