Make WordPress Core

Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#23443 closed enhancement (worksforme)

Change in behaviour of 'wp_signup_location' filter since WordPress 3.5

Reported by: cimmo's profile Cimmo Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.5.1
Component: Multisite Keywords: reporter-feedback
Focuses: Cc:

Description (last modified by ocean90)

Plug-in's author epic:
My plug-in used to attach to the filter 'wp_signup_location' to add a parameter like '?blog_id=7', this to remember where the user clicked 'Register' from.
In example from:

http://localhost/wordpress-ms34/wp-signup.php

to:

http://localhost/wordpress-ms34/wp-signup.php?blog_id=7

Behaviour in WordPress 3.4 MS and earlier:
My plug-in successfully modified the signup location adding the parameter needed.

  1. wp-login.php redirects to the filtered url
  2. wp-signup.php checks if it !is_main_site(), but till 3.4 not passing any parameter to is_main_site was basically always returning true, and !true always false.

Behaviour in WordPress 3.5 MS and later:

  1. wp-login.php redirects to the filtered url
  2. wp-signup.php checks if it !is_main_site(), this returns true (it is not) and then passes and then redirect once more using network_site_url
  3. network_site_url seems removing any parameter added, jeopardizing my filter added earlier.

The exact patch that changed this (correctly probably) is this one:
http://core.trac.wordpress.org/attachment/ticket/22090/22090.diff

My question is: now, how can have my added parameter not filtered out?

thank you
Marco

Change History (13)

#1 @Cimmo
11 years ago

Oh I got it, the problem is not adding *any* parameter, the problem is adding exactly 'blog_id' parameter that fools is_main_site(), so a possible fix is to change to another name.

And I guess even this ticket will be closed as by design.

#2 @ocean90
11 years ago

  • Description modified (diff)
  • Keywords reporter-feedback added

Can you provide a link to your plugin or the part which hooks into wp_signup_location?

#3 @Cimmo
11 years ago

http://plugins.svn.wordpress.org/cimy-user-extra-fields/tags/2.4.1/cimy_uef_register.php

function cimy_change_signup_location($url) {
	global $blog_id, $current_site, $cimy_uef_plugins_dir;

	if ($cimy_uef_plugins_dir == "plugins")
		$attribute = "?blog_id=".$blog_id;
	else
		$attribute = "";

	return "http://" . $current_site->domain . $current_site->path . "wp-signup.php".$attribute;
}

Hook is in:
http://plugins.svn.wordpress.org/cimy-user-extra-fields/tags/2.4.1/cimy_user_extra_fields.php

// add filter to modify signup URL for WordPress MU where plug-in is installed per blog
add_filter('wp_signup_location', 'cimy_change_signup_location');

My workaround applied since v2.4.2 of the plug-in is to not use 'blog_id' as GET paramete after WordPress 3.5

Last edited 11 years ago by Cimmo (previous) (diff)

#4 follow-up: @Cimmo
11 years ago

Also there is no need to edit my description to remove unwanted stuff, are you censoring here? I wrote that sentence because in the past you misbehaved in this bug tracker closing tickets without actually looking at them.
Editing my description won't change the history and makes you look like pathetic.

#5 in reply to: ↑ 4 @helen
11 years ago

Replying to Cimmo:

Editing my description won't change the history and makes you look like pathetic.

The history is there, everyone can still see it if they'd like. It's just distracting from the ticket, as is calling names. I also looked at tickets you've opened here and don't see anything that was closed without a note, so it just overall seems unnecessary.

And now I'll let you all get back to figuring out the actual issue at hand. :)

#6 @Cimmo
11 years ago

  • Resolution set to worksforme
  • Status changed from new to closed

Nevermind, probably this behaviour should have been the one since the beginning, anyway my plug-in version v2.4.2 handle both cases now.

#7 @helen
11 years ago

  • Milestone Awaiting Review deleted

#8 follow-up: @SergeyBiryukov
11 years ago

FWIW, tested the code from comment:3 in 3.4.2, 3.5, 3.5.1, and current trunk (all Multisite installs). Modified the code to always add the blog_id parameter. Didn't notice any changes in behaviour: the parameter is still preserved in 3.5.

#9 in reply to: ↑ 8 @Cimmo
11 years ago

Replying to SergeyBiryukov:

FWIW, tested the code from comment:3 in 3.4.2, 3.5, 3.5.1, and current trunk (all Multisite installs). Modified the code to always add the blog_id parameter. Didn't notice any changes in behaviour: the parameter is still preserved in 3.5.

This is not possible, in 3.4.x or earlier the blog_id=N (N must be different than 1) is not removed, in 3.5.x is removed due to is_main_site() changed behaviour which does another redirect removing blog_id.
As explained in the main description. As said problably NOW the behaviour is ok, but that is still a behavioural change.
Your test doesn't seem consistent with my findings, that means you either used a different code or I am not sure.

#10 follow-up: @Cimmo
11 years ago

wp-signup.php line 25:

if ( !is_main_site() ) {
	wp_redirect( network_site_url( 'wp-signup.php' ) );
	die();
}

In WP 3.4.x that IF with blog_id=2 in the URL for example returns always false.
In WP 3.5.x that IF with blog_id=2 in the URL for example returns true. Then it redirects to itself but without blog_id in the url.

This is what I am talking about.

Last edited 11 years ago by Cimmo (previous) (diff)

#11 @Cimmo
11 years ago

Anyway there is no really need to use my plug-in to reproduce just try to open this url in your browser using WP 3.5.x or greater:

http://localhost/wordpress-ms35/wp-signup.php?blog_id=7

What happens to the blog_id parameter?

Version 1, edited 11 years ago by Cimmo (previous) (next) (diff)

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

Replying to Cimmo:

wp-signup.php line 25:

if ( !is_main_site() ) {
	wp_redirect( network_site_url( 'wp-signup.php' ) );
	die();
}

In WP 3.4.x that IF with blog_id=2 in the URL for example returns always false.
In WP 3.5.x that IF with blog_id=2 in the URL for example returns true. Then it redirects to itself but without blog_id in the url.

I see what you're talking about, but still cannot reproduce.

Here are my exact steps:

  1. Created a new 3.5.1 Multisite install (subdirectory mode).
  2. Pasted this into wp-content/mu-plugins/23443.php:
    <?php
    function change_signup_location_23443( $url ) {
    	global $blog_id, $current_site;
    
    	$attribute = '?blog_id=' . $blog_id;
    
    	return 'http://' . $current_site->domain . $current_site->path . 'wp-signup.php' . $attribute;
    }
    add_filter( 'wp_signup_location', 'change_signup_location_23443' );
    
  3. Created a new site ("Site 1", blog_id is 2).
  4. Went to the new site, logged out, clicked the "Register" link in a sidebar widget.
  5. Ended up on wp-signup.php?blog_id=2 with the registration form.

Anyway there is no really need to use my plug-in to reproduce just try to open this url in your browser using WP 3.5.x or greater:

http://localhost/wordpress-ms35/wp-signup.php?blog_id=7

What happens to the blog_id parameter?

Tried that too. For that URL (with an existing blog_id number, as well as non-existing), is_main_site() still returns true on a clean 3.5.1 install, so the condition in line 25 is not satisfied, and the additional redirect doesn't happen. The blog_id parameter is preserved.

Last edited 11 years ago by SergeyBiryukov (previous) (diff)

#13 @Cimmo
11 years ago

All right, I figured it out, the problem is in MY code, cimy_uef_mu_activation.php placed under /wp-content/mu-plugins (as per my doc) is the one switching the blog when 'blog_id' is encountered.
Then is_main_site() finally returns false. Not on WP 3.4 or earlier though, which I believe was a bug before.
cimy_uef_mu_activation.php is needed because when activating a blog/user you disable all plug-ins, but not the one under 'mu-plugins' directory, which then jeopardize plug-ins like mine that works during the registration.
If you have a better suggestion I'll take it.

So I really apologize for this, it was indeed a change in behaviour in your code, triggered by my code though. I probably still believe you did the right thing (fixing is_main_site() that basically was always returning true with no parameters) and I did not understand it fully at the beginning. Sorry for the report.

Last edited 11 years ago by Cimmo (previous) (diff)
Note: See TracTickets for help on using tickets.