Make WordPress Core

Opened 10 years ago

Closed 10 years ago

Last modified 10 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's profile setsailmedia Owned by: chriseverson's profile chriseverson
Milestone: 3.9 Priority: normal
Severity: normal Version: 3.1
Component: Users Keywords: has-patch commit
Focuses: administration Cc:

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 10 years ago.
sendpasswordcheckboxpersist_v2.diff (777 bytes) - added by chriseverson 10 years ago.
27006.patch (824 bytes) - added by SergeyBiryukov 10 years ago.

Download all attachments as: .zip

Change History (14)

#1 @nacin
10 years ago

  • Component changed from Administration to Users
  • Focuses administration added
  • Keywords needs-patch good-first-bug added

#2 @bcworkz
10 years ago

Possible duplicate? #26962

#3 follow-up: @chriseverson
10 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
10 years ago

  • Owner set to chriseverson
  • Status changed from new to assigned

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
10 years ago

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.

Last edited 10 years ago by chriseverson (previous) (diff)

#6 @SergeyBiryukov
10 years ago

  • Milestone changed from Awaiting Review to 3.9
  • Version changed from 3.8.1 to 3.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
10 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
10 years ago

  • Keywords commit added; good-first-bug removed

#9 @SergeyBiryukov
10 years ago

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

#10 @nacin
10 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

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
10 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.