New user "Send this password to the new user by email" toggle option value not remembered

[setsailmedia]

In this scenario, an administrator is creating a new user account.

He enters the information, ticks the "Send this password to the new user by email" checkbox, and presses the button.

Some error prevents the user account from being created (duplicate email, for example).

The other fields are remembered (user name, name, email, website, etc.) but whether the "Send this password to the new user by email" checkbox was ticked is NOT remembered.

[bcworkz]

Possible duplicate? #26962

[chriseverson]

Just submitted a patch. Next step will be to look into the root of the issue in the checked() function.

[nacin]

Replying to chriseverson:

Just submitted a patch. Next step will be to look into the root of the issue in the checked() function.

Thanks for the patch, chriseverson!

The issue is that checked() is explicit — it wants true === true, not just 'on' == true. (Or more accurately, it converts both to a string and then does a comparison. 'on' does not equal '1'.) checked() isn't the problem here.

Rather, our ugly code that sets this $new_user_send_password variable simply sets it to the value of $_POST['send_password'] if it's set, or false if it's not. Checkboxes without values in the HTML end up receiving 'on' as a value.

This fix is a bit cryptic; simply casting the variable to a boolean would probably solve it in a slightly better way. So, checked( (bool) $new_user_send_password ). Make sense?

[chriseverson]

Replying to nacin:

Replying to chriseverson:

Rather, our ugly code that sets this $new_user_send_password variable simply sets it to the value of $_POST['send_password'] if it's set, or false if it's not. Checkboxes without values in the HTML end up receiving 'on' as a value.

This fix is a bit cryptic; simply casting the variable to a boolean would probably solve it in a slightly better way. So, checked( (bool) $new_user_send_password ). Make sense?

Yeah, that's a much cleaner method. There I was overthinking...

Patch updated.

[SergeyBiryukov]

Introduced in [16518].

For consistency with "Skip Confirmation Email" checkbox in Multisite, we should probably just add value="1":
tags/3.8.1/src/wp-admin/user-new.php#L418​.

edit_user() just checks if $_POST['send_password'] is set, not the actual value:
tags/3.8.1/src/wp-admin/includes/user.php#L179​

[SergeyBiryukov]

Multisite also has "Add Existing User" form, which contains similar fields, but doesn't preserve entered values in case of an error: tags/3.8.1/src/wp-admin/user-new.php#L289​.

Leaving a note here to create a separate ticket for that.

[kirasong]

sendpasswordcheckboxpersist_v2.diff​ looks good.

[SergeyBiryukov]

27006.patch​ implements comment:6, which seems like a more consistent fix to me.

[nacin]

In 27838:

Add a value to the password checkbox when creating a new user so the toggle is remembered.

props chriseverson, SergeyBiryukov.
fixes #27006.

[SergeyBiryukov]

Replying to SergeyBiryukov:

Multisite also has "Add Existing User" form, which contains similar fields, but doesn't preserve entered values in case of an error: tags/3.8.1/src/wp-admin/user-new.php#L289​.

Follow-up: #27606