Make WordPress Core

Opened 9 years ago

Last modified 5 years ago

#31166 new defect (bug)

wpmu_signup_user_notification filter is incorrect

Reported by: johnrom's profile johnrom Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.0
Component: Login and Registration Keywords: dev-feedback
Focuses: multisite Cc:

Description

Simple ticket here,

The wpmu_signup_user_notification filter seems to be filtering the wrong option

	if ( ! apply_filters( 'wpmu_signup_user_notification', $user, $user_email, $key, $meta ) )
		return false;

If I'm thinking correctly, the filter should be filtering a boolean. If two filters are added to this and the first returns false, there is no way for the second filter to recover the $user variable.

This is how I see it working

WP4.1, /wp-includes/ms-functions.php line 919

	if ( ! apply_filters( 'wpmu_signup_user_notification', true, $user, $user_email, $key, $meta ) )
		return false;

Change History (5)

#1 @DrewAPicture
9 years ago

  • Focuses multisite added

#2 @earnjam
9 years ago

  • Version changed from 4.1 to 3.0

Looks like it's been this way since MU.

#3 @SergeyBiryukov
9 years ago

Same for wpmu_signup_blog_notification.

Technically these filters can still be used, they just have to return a truthy or falsy value.

Fixing this would break backwards compatibility. We could probably introduce new filters and deprecate these ones, otherwise this seems like a wontfix.

#4 @johnrom
9 years ago

Since it is only used during user creation, I don't think a second filter will hurt, performance-wise. Deprecation would be fine. For forward-compatibility, another plugin could set the username to false and break mission-critical functionality. If I'm not mistaken, these are the only places the username is exposed on a filter between user signup and the redirection when creating a user in multisite.

#5 @jeremyfelt
9 years ago

  • Keywords dev-feedback added

It could be tough to really deprecate the filter as you'd always need to account for it.

A new filter would likely go immediately after the current filter. Then if using the new filter, you would need to add_filter( 'wpmu_signup_blog_notification', '__return_true', 999 ); to try and overrule any previous use before hooking in to new_wpmu_signup_blog_notification.

Part of me wants to wontfix. While somewhat annoying, if a plugin has a notification that really needs to fire, a if ( ! $user ) { // find user by email or key from signups table } could do the trick.

I could go either way though. :)

Note: See TracTickets for help on using tickets.