Make WordPress Core

Opened 13 years ago

Closed 12 years ago

Last modified 12 years ago

#27006 closed defect (bug) (fixed)

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

Reported by: setsailmedia Owned by: chriseverson
Priority: normal Milestone: 3.9
Component: Users Version: 3.1
Severity: normal Keywords: has-patch commit
Cc: Focuses: administration

Description

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.

Attachments (3)

sendpasswordcheckboxpersist.diff (776 bytes ) - added by chriseverson 13 years ago.
sendpasswordcheckboxpersist_v2.diff (777 bytes ) - added by chriseverson 13 years ago.
27006.patch (824 bytes ) - added by SergeyBiryukov 12 years ago.

Download all attachments as: .zip

Change History (14)

#1 @nacin
13 years ago

  • Component AdministrationUsers
  • Focuses administration added
  • Keywords needs-patch good-first-bug added

#2 @bcworkz
13 years ago

Possible duplicate? #26962

#3 follow-up: @chriseverson
13 years ago

  • Keywords has-patch added; needs-patch removed

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

#4 in reply to: ↑ 3 ; follow-up: @nacin
13 years ago

  • Owner set to chriseverson
  • Status newassigned

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?

#5 in reply to: ↑ 4 @chriseverson
13 years ago

Replying to 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?

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

Patch updated.

Version 0, edited 13 years ago by chriseverson (next)

#6 @SergeyBiryukov
13 years ago

  • Milestone Awaiting Review3.9
  • Version 3.8.13.1

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

#7 follow-up: @SergeyBiryukov
13 years ago

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.

#8 @kirasong
12 years ago

  • Keywords commit added; good-first-bug removed

#9 @SergeyBiryukov
12 years ago

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

#10 @nacin
12 years ago

  • Resolutionfixed
  • Status assignedclosed

In 27838:

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

props chriseverson, SergeyBiryukov.
fixes #27006.

#11 in reply to: ↑ 7 @SergeyBiryukov
12 years ago

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

Note: See TracTickets for help on using tickets.