WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 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
Milestone: 3.9 Priority: normal
Severity: normal Version: 3.1
Component: Users Keywords: has-patch commit
Focuses: administration Cc:
PR Number:

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

Download all attachments as: .zip

Change History (14)

#1 @nacin
6 years ago

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

#2 @bcworkz
6 years ago

Possible duplicate? #26962

#3 follow-up: @chriseverson
6 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
6 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
6 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 6 years ago by chriseverson (previous) (diff)

#6 @SergeyBiryukov
6 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
6 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 @mikeschroder
6 years ago

  • Keywords commit added; good-first-bug removed

#9 @SergeyBiryukov
6 years ago

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

#10 @nacin
6 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
6 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.