GNOME Bugzilla – Bug 321567
usability problems with Change Password in about-me capplet
Last modified: 2010-09-30 11:24:42 UTC
The change password dialog in the about-me capplet seems messy. If you type a password that gets rejected (lets say it is too short), and you start typing in a new password in "New Password" or "Retype New Password" it suddently says "Please type the password again, it is wrong". This message seems confusing when I'm in the middle of typing in a new password. Perhaps the message could say something like "The new password values are not the same" so it doesn't imply something is wrong when the user may be in the middle of just changing the values. Also, some of the "error" messages don't give you a clear message that you have to retype the new password again. If I came to this dialog for the first time and entered my old password and my new password twice, clicked the "Change Password" button and it says "Password is too simple" I might not understand that this is an error message and I'm expected to retype the two fields and try again. Perhaps this is a nit. The most serious problem I notice is in the read_everything() function in gnome-about-me-password.c. If the message doesn't contain one of the words passed into the function via the "needle" argument, the while loop never breaks and the dialog just hangs in the while loop reading EOF over and over. The loop should break when EOF is reached to avoid hanging if an unexpected message is returned. If an unrecognized message is passed in, why doesn't the update_password function just spit out whatever message it received. I understand you might be trying to rephrase the messages so that the about-me dialog shows consistant error messages on all systems and/or to ensure that the message fits into the dialog. However, if you get an unexpected message, it isn't very good to hang and not display the message.
More seriously, the way the about me password dialog works is not really compatible with the Q-and-A approach used by pam. For informational purposes, I'll attach the patch we use in Fedora to bypass the builtin password dialog, and use the Red Hat-specific usermode framework instead (by spawning /usr/bin/userpasswd)
Created attachment 54834 [details] [review] the patch
Will this bug fixed in upcoming GNOME 2.14 ?
i don't have time to fix this for GNOME 2.14, currently i'm very busy working on my master thesis. I just looked in Debian and found that it has userpasswd, i thought that only Red Hat Enterprise and Fedora had userpasswd, it seems debian also has it (package usermode in unstable), if you can tell me that that SuSE and /or Mandriva has userpasswd i will review and integrate this patch.
*** Bug 328754 has been marked as a duplicate of this bug. ***
Mandriva has usermode & userpasswd. On Mandriva this is required (among others) by gnome-session, so it is always installed.
I would suggest splitting the process in two, as this would be more PAM-compatible: - First enter your current password for authentication - If authentication was successful, set the two new password fields to sensitive. I've made a suggestion to a new password-dialog interface. It can be found at http://joh.deworks.net/password-dialog/screenshots/password-dialog.png
Anyone working on this bug, or is it dead? I'm willing to work on this if I can get some feedback on my previous comment and the green light.
i think it is ok. You can work on this and i'll comment on the patch as soon as possible.
Ok, I've been tinkering with this for a couple of weeks and this is what I've come up with. I've almost done a complete rewrite of the original dialog - below is a summary of the changes: - A completely new UI - Replaced forkpty with GSpawn for spawning /usr/bin/passwd - Replaced backend communication system (FILEs and file descriptors) with GIOChannels - Replaced g_timeout-based child watcher with GChildWatch I know I was only supposed to fix the UI part, but I felt that rewriting some of the underlying code would simplify the process (I had a hard time digging through all of the low-level UNIX code). UI Notes: I've kept the old dialog in the same glade file as "change-password-old" The lightbulb icons has been replaced by the stock_lock and stock_lock-open icons. This lock indicates wether the user has authenticated or not. I was unsure whether to include them in the capplet or to use the stock icons. Either way, they are not very theme friendly (I think). I've included them for now. I designed the status message field after the one in gnome-screensaver. It is updated whenever an error occurs or to messages guiding the user through the process. It is updated instantly whenever any of the new password entries change, which might be a bit confusing. Maybe adding a (short!) timeout to these would help, as the original dialog had (only it was rather long - 0.5s). It is set to wrap, and center-aligned. This poses a problem if the message changes from a short one spanning one line, to a longer one, spanning two lines (or vice versa). The height of the dialog will then jump up and down, which is quite inelegant. I have not found any solution to this problem yet. I've tried to make the UI more HIG compliant. For instance, I have followed the guideline about authentication dialogs, so that that hitting <enter> on either of the entries focuses or activates the next logical widget in the process (i.e. hitting enter in the new-password entry will move focus to the retyped-password entry). I'm sure there are more guidelines to follow though :) Fixed a bug in the old dialog, including: - passdlg_process_response() always tries to kill() using the PID of the backend, even if the backend has already exited. This can, in rare cases, cause it to kill another process. (I'm sure there are other bugs which have been fixed as well during the rewrite, and I'm sure the rewrite will introduce new ones! I hope it won't though ;) Todo (probably before inclusion): Set status message to have a fixed height of 2 lines (if possible, to avoid the dialog height jumping up and down) Check that the UI is HIG compliant. Status message is updated instantly whenever any of the new password entries change - might pose a usability problem. Check spelling of the status messages Are the status messages translatable? Should text refering to labels be dynamically read from the widgets? I.e. "Change Password button ..." Remove debug messages (There are loads) Future improvements: Use a helper program instead of /usr/bin/passwd as the backend. Maybe the redhat helpers mentioned here already? Also, should we support other PAM configurations than the plain-old-password? Add an inactivity timeout for security (?) If user authenticates and leaves the dialog open "Shake" window when authentication fails, like in gnome-screensaver and gdm (?) That's all! The dialog works quite well (as far as I can tell from my testing) - it even handles the case where the users password changes after he has authenticated! I'll put up the patches and full versions of the files, including images, at http://joh.deworks.net/gnome-about-me-password/ Looking forward to your comments!
Created attachment 64247 [details] [review] UI patch
Created attachment 64248 [details] [review] Dialog patch
Created attachment 64249 [details] [review] Makefile.am patch
The (stock) images used can be found at http://joh.deworks.net/gnome-about-me-password/files/
Thank you very much i'll start to review the patches and the UI as soon as possible. That means this week hopefully.
(In reply to comment #15) > Thank you very much i'll start to review the patches and the UI as soon as > possible. That means this week hopefully. > Great! (Wow, that reply was quick :)
I have not tried anything yet. But having checked the interface it seems that there is too much text in there. I'm also not sure that the text in the bottom is needed. Windows does not provide anything like this and I think that people don't find it so hard to use it ... I also saw MacOs and doesn't provide anything like this (at least in the screenshots i found). I don't know. I will try to look at the code tomorrow. I'm also in the process of changing the whole UI to solve some of the pending issues.
(In reply to comment #17) > I have not tried anything yet. But having checked the interface it seems that > there is too much text in there. Ok, I see your point. Removing the explanatory text is no problem at all :) > > I'm also not sure that the text in the bottom is needed. Windows does not > provide anything like this and I think that people don't find it so hard to use > it ... > I also saw MacOs and doesn't provide anything like this (at least in the > screenshots i found). Actually, Mac OS X has something like this (I've got it running on my PowerBook). On an error, the dialog prints a red error message under the entry field where the problem occured - plus a red arrow pointing at the field. See http://joh.deworks.net/gnome-about-me-password/osx/ for screenshots. Actually, something like this would be really nice to have in GTK (if it already is, enlighten me :), as it makes it really obvious for the user to see which field caused the error. Maybe we should just use the status message for the errors alone? Maybe the text should be bold and bright red (+ some a11y stuff)? Maybe we should skip checking that the two passwords are equal and leave that to passwd? > > I don't know. I will try to look at the code tomorrow. I'm also in the process > of changing the whole UI to solve some of the pending issues. > Which UI, the old one or the new? Which pending issues?
> Actually,something like this would be really nice to have in GTK (if it > already is, enlighten me :) I don't know, but maybe a workaround could be used. (See screenshot attached, it might be a little too hacky, though) > Which UI, the old one or the new? Which pending issues? I'm talking about the old UI. Mainly those of people that say that gnome-about-me is a bad clone of the address book editor and those that say that there are too many details in it. I sent the proposal of the new UI along with your UI for changing passwords to the usability mailing list. These are some of the comments related to the password dialog (i don't necesarily agreen with them): *) Some suggested to have a checkbox that would render the passwords redable. ( I don't agree with this). *) It seems that disabling things (like the "change password") button until the moment that the user has authenticated correctly and both passwords are OK is confusing to users ... It also seems confusing to disable the "new password" and "verify password" text boxed until authentication has been performed. *) Other user suggested separating authentication into a separate dialog
Created attachment 64609 [details] Hack to "clone" the change password functionality in MacOS
Created attachment 64610 [details] Hack to "clone" the change password functionality in MacOS What do you think about this approach, is a bad hack but it might be enough.
(In reply to comment #19) > > Actually,something like this would be really nice to have in GTK (if it > > already is, enlighten me :) > > I don't know, but maybe a workaround could be used. (See screenshot attached, > it might be a little too hacky, though) Yeah, it's a bit hacky I think, but it might just work. It takes up some space between each of the GtkEntry widgets though (but so it does in OS X). Maybe centering the error-message so it's not as cramped up to the label. IMHO this should be a high-level, standard GTK widget, perhaps also with a themable icon to the right of the text field with the invalid input (like in OS X). Something like that would be very useful for applications which need to validate user input. (I also looked at your screenshots from your post at usability list - the new UI is a lot better than the old one IMHO, though I think the gam-v2-0 dialog is a bit too high. Also, maybe the picture should appear to the left of the personal info, as it does on driving certificates and some bank cards?) > > > Which UI, the old one or the new? Which pending issues? > > I'm talking about the old UI. Mainly those of people that say that > gnome-about-me is a bad clone of the address book editor and those that say > that there are too many details in it. I see their point, thought personally I think it works fine :P In OS X, there's a button under each account leading to the address book editor. I've put up some screenshots at http://joh.deworks.net/gnome-about-me/osx/ I hope they can be useful, as the Mac guys tend to be quite good when it comes to designing UIs with usability in mind... (Not that we should copy them :P) > > I sent the proposal of the new UI along with your UI for changing passwords to > the usability mailing list. > > These are some of the comments related to the password dialog (i don't > necesarily agreen with them): > > *) Some suggested to have a checkbox that would render the passwords redable. ( > I don't agree with this). Me neither. Passwords are *secret* by nature, and the dialog shouldn't make it easier for an eavesdropper to obtain the password. > > *) It seems that disabling things (like the "change password") button until the > moment that the user has authenticated correctly and both passwords are OK is > confusing to users ... > > It also seems confusing to disable the "new password" and "verify password" > text boxed until authentication has been performed. I can understand how disabling the new password fields until they contain valid input might be confusing. However, I don't understand how it's confusing to disable the rest of the UI until the user has typed in his current password. The explanatory text explains this process (which we're about to remove :P), and IMO it makes perfect sense - "In order to change your password, you must tell me your current password first so I can ensure that you are really who you claim to be". This is also how it's done in passwd - it doesn't let you type in your new passwords before it has verified that your current password is correct. A compromise might be to enable the rest of the UI, but as soon as any of it (the part which requires authentication) recieves focus (and the user has not authenticated), a notification bubble or some other text appears ("somewhere in the UI") which explains the user that he has to authenticate first. This might be even more confusing though :/ > > *) Other user suggested separating authentication into a separate dialog > This I strongly oppose, and is one thing I didn't like about /usr/bin/userpasswd. Opening a dialog asking the user to type in his password and the *closing it* afterwards, just to open a new one is *very* confusing IMHO. Another solution might be to initially hide the part of the UI which requires authentication. When the user types in his password and hits Authenticate, the hidden part appears (or "fades in"). We might then also hide the authentication-part, but we should *never* present the user with multiple dialogs, popping up one after another! Have you had the time to look at some of the code, btw? I assume you got it to build and run alright? Great feedback!
OK, here is a batch of comments. Sorry for the delay. Some cosmetic things: in general the variables used in a function should be declared at the top of the function, for example, the order should be changed in io_watch_stdout. There are a bunch of g_debug statements that should be removed. You have to provided a ChangeLog for the changes. Correct me if i'm wrong in this: i think that when you spawn passwd you are not setting the locale anywhere, this means that if i have es_ES as locale the messages will be given in spanish and the dialog will fail to do anything useful. Some other things: There is a very good message in the usability mailing list about the password dialog (in the sense that it provides useful advices not blue-sky ones). See: http://mail.gnome.org/archives/usability/2006-May/msg00040.html The UI issues can be corrected once this code is commited, probably the messages will changed when you take into account the suggestions in the message linked. Right now the UI is ok for most systems, granted that it only works for a certain PAM configuration but right now i think is enough, in following iterations it can be improved. Right now i think the code should be commited once the two things i said are addressed. I also think the interactivity time out should be handled. ---------------------------------------------- One question: Is it possible to separate all the code that enables the dialog to talk to passwd and generalized it some way? This is what i will be trying to do: There have been several requests to let the user change his/her real name from the dialog. The problem is that you have to either parse /etc/login.conf or try to change the name to see if it is allowed. I think it would be easier to just try to change the user name to see if we are allowed to do it. So i need to spawn and talk to chfn.
(In reply to comment #23) > OK, here is a batch of comments. Sorry for the delay. > > Some cosmetic things: > > in general the variables used in a function should be declared at the top of > the function, for example, the order should be changed in io_watch_stdout. Done. I've tried to locate all the declarations not following this. Let me know if you find more. > > There are a bunch of g_debug statements that should be removed. Removed. > > You have to provided a ChangeLog for the changes. Ok, adding in latest patch. > > Correct me if i'm wrong in this: i think that when you spawn passwd you are not > setting the locale anywhere, this means that if i have es_ES as locale the > messages will be given in spanish and the dialog will fail to do anything > useful. This is something I've been unsure about. If we pass an empty array as the environment to g_spawn_async_with_pipes, I thought childs environment would be empty, and the locales then set to the C default. See my comment in spawn_passwd. > > Some other things: > There is a very good message in the usability mailing list about the > password dialog (in the sense that it provides useful advices not blue-sky > ones). See: http://mail.gnome.org/archives/usability/2006-May/msg00040.html Very good points, though I don't agree to all of them ;) I'll post a reply ASAP. > > The UI issues can be corrected once this code is commited, probably the > messages will changed when you take into account the suggestions in the message > linked. > > Right now the UI is ok for most systems, granted that it only works for a > certain PAM configuration but right now i think is enough, in following > iterations it can be improved. I agree, there was some discussion here about using the redhat helpers. Maybe these should/could be used at a later iteration? > > Right now i think the code should be commited once the two things i said are > addressed. I also think the interactivity time out should be handled. Ok, so you think the last UI and code should be commited as it is right now? (after I've fixed the cosmetics, removed the debug messages, added a ChangeLog entry and are sure the locales are set to C) What interactivity timeout? Are you referring to the security inactivity timeout? Should this be added before the first commit? I'll attach the patch shortly. > > ---------------------------------------------- > > One question: Is it possible to separate all the code that enables the dialog > to talk to passwd and generalized it some way? This is what i will be trying to > do: > > There have been several requests to let the user change his/her real name > from the dialog. The problem is that you have to either parse /etc/login.conf > or try to change the name to see if it is allowed. I think it would be easier > to just try to change the user name to see if we are allowed to do it. So i > need to spawn and talk to chfn. > Sure, that shouldn't be a problem. The spawner, IO watcher and IO queue are definitely suited for the job of talking to chfn. Where would this code be placed? As a seperate object in about-me, or some other library?
Created attachment 65257 [details] [review] UI patch v2 (removed old dialog)
Created attachment 65258 [details] [review] Dialog patch v2
Created attachment 65259 [details] [review] ChangeLog patch
(In reply to comment #24) > > I agree, there was some discussion here about using the redhat helpers. Maybe > these should/could be used at a later iteration? Sure, i was thinking about a later iteration, not this one. > > > > > Right now i think the code should be commited once the two things i said are > > addressed. I also think the interactivity time out should be handled. > > Ok, so you think the last UI and code should be commited as it is right now? > (after I've fixed the cosmetics, removed the debug messages, added a ChangeLog > entry and are sure the locales are set to C) yeap, the UI can be worked out once this is commited. > > What interactivity timeout? Are you referring to the security inactivity > timeout? Should this be added before the first commit? > No, we can commit it now and and add it afterwards. > I'll attach the patch shortly. Cool > Sure, that shouldn't be a problem. The spawner, IO watcher and IO queue are > definitely suited for the job of talking to chfn. Where would this code be > placed? As a seperate object in about-me, or some other library? > Yes, as a separate object, i don't think it deserve a library as it is quite small. Do you have commit access to the repository or should i commit it?
(In reply to comment #28) > (In reply to comment #24) > ... snip ... > > yeap, the UI can be worked out once this is commited. Great! > > > > > What interactivity timeout? Are you referring to the security inactivity > > timeout? Should this be added before the first commit? > > > No, we can commit it now and and add it afterwards. Ok, good. > > > I'll attach the patch shortly. > > Cool > > > Sure, that shouldn't be a problem. The spawner, IO watcher and IO queue are > > definitely suited for the job of talking to chfn. Where would this code be > > placed? As a seperate object in about-me, or some other library? > > > Yes, as a separate object, i don't think it deserve a library as it is quite > small. I was thinking more like integrating it into some library, though I'm not sure which one... > > Do you have commit access to the repository or should i commit it? > I don't, so go ahead and commit it. I would love a CVS account though :)
ok, i have just commited the code to CVS, now we can start to tackle the small details. Thank you very much. I'll keep the bug open until we finish everything, ok?
(In reply to comment #30) > ok, i have just commited the code to CVS, now we can start to tackle the small > details. Thank you very much. Great! Glad I can help! > > I'll keep the bug open until we finish everything, ok? > Sure. I replied to Matthew Paul Thomas' post on the usability list, but got no replies. IMO I think we should discuss things further on the usability list before we start working on the other problems...
Is this bug fixed in GNOME 2.18 ? If not - please also fix bug #336872 with the same patch, if possible
(In reply to comment #32) > Is this bug fixed in GNOME 2.18 ? If not - please also fix bug #336872 with the > same patch, if possible > Hmm, this bug (#321567) has already been implemented in 2.18 so I think #336872 is actually a bug in my code (and thus a bug in my patch for #321567). I'll have a look at it as soon as possible (earliest at Tuesday next week.)
I'll close this. about-me is deprecated, and accountsdialog offers the ability to change passwords already.