Created attachment 249911 [details] [review]
add positive confirmation to the password dialog

Created attachment 249912 [details] [review]
add positive confirmation to the accounts dialog

Created attachment 250926 [details] [review]
add positive confirmation to the password dialog

Rebased to master.

Created attachment 250927 [details] [review]
add positive confirmation to the add user dialog

Rebased to master.
Positive confirmation also for the enterprise part.

Review of attachment 250927 [details] [review]:

Get this critical:

(gnome-control-center:12365): GLib-CRITICAL **: g_source_remove: assertion 'tag > 0' failed

when I:

1) Add User
2) Type 'Blue' in Full Name
3) Tab to Username

Also, seems like commit message needs to better describe the changes. Did you mean to stop setting
tooltips on the icons?

::: panels/user-accounts/um-account-dialog.c
@@ -324,3 @@
         entry = gtk_bin_get_child (GTK_BIN (self->local_username));
         if (tip) {
-                set_entry_validation_error (GTK_ENTRY (entry), tip);

No exclam icon secondary entry icon is set when I choose an invalid user name. Is that by design? If so, please mention in commit message so intent and reasoning is clear.

@@ +399,3 @@
         entry = gtk_bin_get_child (GTK_BIN (self->local_username));
         clear_entry_validation_error (GTK_ENTRY (entry));
+        gtk_dialog_set_response_sensitive (GTK_DIALOG (self), GTK_RESPONSE_OK, FALSE);

Seems this leads to flicker and frustration. The user cannot complete the dialog for one second after filling in the user name? Clicking enter during this time does nothing.

You may need to trigger validation early if GTK_RESPONSE_OK is clicked during timeout.

@@ +407,3 @@
+local_name_timeout (UmAccountDialog *self)
+{
+        dialog_validate (self);

You need to:

self->local_name_timeout_id = 0

@@ +422,3 @@
+                g_source_remove (self->local_name_timeout_id);
+                self->local_name_timeout_id = 0;
+static gboolean

What's going on here? Are you sure you want to be checking local_username_timeout_id but clear local_name_timeout_id (ie: different timeouts)?

@@ -420,3 @@
-        if (strlen (verify) != 0 && strcmp (password, verify) != 0) {
-                gtk_label_set_label (GTK_LABEL (self->local_hint), _("Passwords do not match."));
-                set_entry_validation_error (GTK_ENTRY (self->local_verify), _("Passwords do not match"));

I no longer get a 'Passwords do not match' message when I type a bad Confirm password. The 'Add' button is simply disabled.

@@ +557,2 @@
         self->local_name = widget;
+        self->local_name_timeout_id = 0;

Unecessary. all fields are initially set to zero, and we make this assumption for other fields.

@@ +1578,3 @@
+                g_source_remove (self->local_username_timeout_id);
+                self->local_username_timeout_id = 0;
+        }

Ditto, seems like mismatched variables here.

Review of attachment 250926 [details] [review]:

Some of the same issues as the other patch.

In addition when pressing tab from the 'New Password' field I go to 'Cancel' rather than to the 'Confirm New Password' field. This happens regardless of whether the password is strong enough or not.

::: panels/user-accounts/pw-utils.c
@@ +118,3 @@
                 return C_("Password hint", "Mix uppercase and lowercase and use a number or two.");
+        default:
+                return C_("Password hint", "Good password! Adding more letters, numbers and punctuation will make it stronger.");

I think having an exclam here is a bit sarcastic. Like "Oh my god, you finally managed to find a good password". I know that's how I felt :)

::: panels/user-accounts/um-password-dialog.c
@@ -295,3 @@
-                                             _("The passwords do not match."));
-                        set_entry_validation_error (GTK_ENTRY (um->verify_entry),
-                                                    _("Passwords do not match"));

I no longer get a message that the passwords don't match.

@@ +324,3 @@
         clear_entry_validation_error (GTK_ENTRY (entry));
+        clear_entry_validation_error (GTK_ENTRY (um->verify_entry));
+        gtk_widget_set_sensitive (um->ok_button, FALSE);

I think this is a bad approach which prevents clicking OK, or pressing enter after typing passwords.

Thank you for the reviews!

These criticals coming from removing wrong source as you've mentioned bellow. Removing tooltips, removing "Passwords do not match", adding "Good password!" are by design. 

Rest of your notes I'll try to fix and also find better way to prevent clicking OK...

Created attachment 251704 [details] [review]
add positive confirmation to the password dialog

Timeout decreased to be compromise of flickering hints and waiting for submit.
Reverted "Passwords do not match." hint.
Removed unnecessary initialization and other fixes.

Created attachment 251705 [details] [review]
add positive confirmation to the add user dialog

Same as previous patch.

Changes discussed with aday.

Review of attachment 251704 [details] [review]:

Bug: A checkmark appears next to 'Current Password' even when I've typed the wrong password.

I'm still seeing the case where I can't click Enter until the timeout completes. But maybe this is acceptable, and I'm not trying to make this a dealbreaker. I just find it awkward. Maybe a better solution will be found later.

Other than that this patch looks good to go.

Review of attachment 251705 [details] [review]:

I see this critical:

(gnome-control-center:3510): GLib-GObject-CRITICAL **: g_object_notify: assertion 'G_IS_OBJECT (object)' failed

Bug: When I type password in 'Password' and press <tab> immediately after typing the last character of the password, I'm taken to the 'Enterprise Login' button instead of the 'Confirm Password' field. This is awkward and frustrating.

The same applies to pressing <enter> immediately after filling out the "Current Password" field. The dialog does not complete, as noted in the other patch.

Other than that the patch looks good.

(In reply to comment #11)
> Review of attachment 251705 [details] [review]:
> 
> I see this critical:
> 
> (gnome-control-center:3510): GLib-GObject-CRITICAL **: g_object_notify:
> assertion 'G_IS_OBJECT (object)' failed
> 
> Bug: When I type password in 'Password' and press <tab> immediately after
> typing the last character of the password, I'm taken to the 'Enterprise Login'
> button instead of the 'Confirm Password' field. This is awkward and
> frustrating.

This same thing happens in the 'Change paswsord' dialog, if you touch type a password rapidly and immediately follow with <tab> you go to the "Cancel" button instead of 'Verify New Password'.

(In reply to comment #11)
> I see this critical:
> 
> (gnome-control-center:3510): GLib-GObject-CRITICAL **: g_object_notify:
> assertion 'G_IS_OBJECT (object)' failed

This is a gtk+ bug #706152

Created attachment 251938 [details] [review]
user-accounts: Don't disable the verify password entry

This completely breaks pressing <TAB> to get to the next
control in the dialog, especially when immediately typing
a password.

Created attachment 251939 [details] [review]
user-accounts: activates_default = TRUE on add account password entries

Created attachment 251940 [details] [review]
user-accounts: Validate dialog immediately when enter is pressed

This allows users to touch type passwords, press <ENTER> without
waiting for the pasword timeout to clear.

Attachment 251938 [details] pushed as 7b3746a - user-accounts: Don't disable the verify password entry
Attachment 251939 [details] pushed as 6d21f8f - user-accounts: activates_default = TRUE on add account password entries
Attachment 251940 [details] pushed as 7ae6814 - user-accounts: Validate dialog immediately when enter is pressed

Fixed up for comments, and pushed. Freeze is coming up, and neither Ondrej or I will be here.

Excellent! Congratulations Ondrej.