Rewrite query vars are stripped from custom taxonomy page URLs via redirect_canonical()

[gnoodl]

In wp-includes/canonical.php, version 3.1 changed lines 155-177 from

if ( $term_count <= 1 && !empty($obj->term_id) && ( $tax_url = get_term_link((int)$obj->term_id, $obj->taxonomy) )
		&& !is_wp_error($tax_url) && $redirect['query'] ) {

    /* Not important */
    
	$tax_url = parse_url($tax_url);
	if ( ! empty($tax_url['query']) ) { // Custom taxonomies may only be accessable via ?taxonomy=..&term=..
		parse_str($tax_url['query'], $query_vars);
		$redirect['query'] = add_query_arg($query_vars, $redirect['query']);
	} else { // Taxonomy is accessable via a "pretty-URL"
		$redirect['path'] = $tax_url['path'];
	}
}

to (lines 153-174)

if ( $term_count <= 1 && !empty($obj->term_id) && ( $tax_url = get_term_link((int)$obj->term_id, $obj->taxonomy) ) && !is_wp_error($tax_url) ) {
	if ( !empty($redirect['query']) ) {

        /* Not important */
	    
	}
	$tax_url = parse_url($tax_url);
	if ( ! empty($tax_url['query']) ) { // Custom taxonomies may only be accessable via ?taxonomy=..&term=..
		parse_str($tax_url['query'], $query_vars);
		$redirect['query'] = add_query_arg($query_vars, $redirect['query']);
	} else { // Taxonomy is accessable via a "pretty-URL"
		$redirect['path'] = $tax_url['path'];
	}
}

The difference here is that in the new version, the $redirect['path'] is always set to the taxonomy path, eliminating any query vars added via custom rewrite rule, regardless of whether there are query vars in $redirect['query'] or not.

Just discovered this issue on my site as well. In my research, I found a few differences that seem to be causing the "issue".

1) In the code just above what @gnoodl mentioned, in 3.0.5, the following code results in a $term_count of 0:

$term_count = 0;
foreach ( array('category__in', 'category__not_in', 'category__and', 'post__in', 'post__not_in',
'tag__in', 'tag__not_in', 'tag__and', 'tag_slug__in', 'tag_slug__and') as $key )
	$term_count += count($wp_query->query_vars[$key]);

In 3.1, the new code results in a $term_count of 1:

$term_count = 0;
foreach ( $wp_query->tax_query->queries as $tax_query )
	$term_count += count( $tax_query['terms'] );

2) In the if condition that @gnoodl mentioned, 3.0.5 includes a check for $redirectquery?, 3.1 does not.

---

In my case, I'm using a custom query variable. Perhaps there is another solution to my problem, but it worked in 3.0.5 and this is the code that causes it to no longer work ;)

[15705] is where the conditional was altered.

A test case would be helpful here. What is an example URL that is failing? What does the registration code for the taxonomy look like?

Here's what I am using. Worked in 3.0.5, not in 3.1:

// create the custom taxonomies
function btf_create_taxonomies() {
	register_taxonomy('textures', 'post', array('hierarchical' => false, 'label' => 'Texture Categories', 'query_var' => true, 'rewrite' => true));
	register_taxonomy('wallpapers', 'post', array('hierarchical' => false, 'label' => 'Wallpaper Categories', 'query_var' => true, 'rewrite' => true));
}
add_action('init', 'btf_create_taxonomies', 0);

// add the custom query var
function btf_query_vars($query_vars) {
	$query_vars[] = 'sort';
	return $query_vars;
}
add_filter('query_vars', 'btf_query_vars');

// I had some issues with custom query vars and paging, 
// so I wrote this to correct the query string values
function btf_request($query_string) {
	if (isset($query_string['name']) && $query_string['name'] === 'page'
			&& isset($query_string['page'])) {
		unset($query_string['name']);
		$query_string['paged'] = str_replace('/', '', $query_string['page']);
	}

	$sort = array('date', 'popularity', 'comments', 'downloads', 'rating', 'votes');
	if (isset($query_string['name']) && isset($query_string['category_name'])
			&& btf_ends_with('/sort', $query_string['category_name'])) {
		if (strpos($query_string['name'], '-') > 0) {
			$valid = explode('-', $query_string['name']);
			$s = $valid[0];
		}
		else {
			$s = $query_string['name'];
		}

		if (in_array($s, $sort)) {
			$parts = explode('/', $query_string['category_name']);
			$query_string['sort'] = $query_string['name'];
			$query_string['category_name'] = $parts[0];
			unset($query_string['name']);
		}
	}
	return $query_string;
}
add_filter('request', 'btf_request');

I hope the btf_request function doesn't over-complicate things. Basically, what I want is to be able to have:

• mysite.com/custom-taxonomy/ - default listing
• mysite.com/custom-taxonomy/sort/downloads - sort by downloads
• mysite.com/custom-taxonomy/sort/downloads/page/2 - sort by downloads, page 2

The code I have above worked fine in 3.0.5. If there is a better way to achieve this, I'm happy to try a different approach.

Mine looks very similar to emartin's except I'm using rewrite rules.

Taxonomy code

register_taxonomy('my_taxonomy', 'custom_post_type', array(
    'label'     => __('My Taxonomy Name'),
    'public'    => true,
    'rewrite'   => array('slug' => 'my_slug_prefix'),
    'query_var' => 'my_taxonomy_var'
));

Added query vars (using the query_vars filter)

$queryVars[] = 'criteria';
return $queryVars;

Rewrite Rules (using the generate_rewrite_rules action hook)

$rules = array(
    'my_slug_prefix/(.+)/(.+)/(asc|desc)' => sprintf(
        'index.php?taxonomy=my_taxonomy&my_taxonomy_var=%s&criteria=%s&order=%s',
        $rewrite->preg_index(1), $rewrite->preg_index(2), $rewrite->preg_index(3)),
    'my_slug_prefix/(.+)/(.+)' => sprintf(
        'index.php?taxonomy=my_taxonomy&my_taxonomy_var=%s&criteria=%s',
        $rewrite->preg_index(1), $rewrite->preg_index(2))
);
$rewrite->rules = $rules + $rewrite->rules;

Like emartin's, mine worked fine in 3.0.5. Any matching URL is now redirected to the default taxonomy page via canonical.php line 172

$redirect['path'] = $tax_url['path'];

[15705] is where the conditional was altered.

That reverted [15462], which was hiding the issue by only processing requests with $_GET variables present.

This particular change is rooted in the fact that non-category, non-tag taxonomies got a canonicalisation treatment.

My initial thought pattern here, is that the function should check to see what $query_vars are present, ie. if anything other than taxonomy & $taxonomy_query_var are present in the rewrite, skip canonicalising based on the term path.

My initial thought pattern here, is that the function should check to see what $query_vars are present, ie. if anything other than taxonomy & $taxonomy_query_var are present in the rewrite, skip canonicalising based on the term path.

attachment 16627.diff added

• The above idea in code.

The other option is to cripple the canonicalisation for taxonomies for 3.1.x by adding back in the check for $_GET variables present, that'll at least cause canonical to not apply to pure rewrite rules, but as soon as a $_GET variable is added to the mix, it'll kick in again.. as it should've done so in 3.0. and leave above patch for a 3.2 fix.

What's the other option look like, in patch form? :-)

How about doing a redirect only if there are $_GET variables present that are not registered with $wp->add_query_var() ?

The alternate to "cripple" it, is to re-instate [15462] - specifically, checking to see if a query is set. Which is a better option for 3.1.1, leaving a proper fix to 3.2

Putting $redirectquery? back for 3.1.1 sounds good to me.

(In [17549]) Reinstate [15462] for 3.1; Prevents canonical redirects for custom rewrite rules for taxonomies. See #16627

This was never dealt with in trunk

Related: #17174

With the re-instated patch, term URLs are not canonicalized (though they probably weren't before). If you are using pretty permalinks, then the $redirect['query'] will never be set (looking at http://core.trac.wordpress.org/browser/tags/3.1.2/wp-includes/canonical.php#L153).

This means if the $tax_url is different to the current URL, it will not be redirected. E.g. /tag/Apple/ should redirect to /tag/apple/, but won't be. Removing the $redirect['query'] check fixes the canonical redirect in my case.

With the re-instated patch, term URLs are not canonicalized

Yep, For 3.1.x the decision was to change as little code as possible to restore the previous functionality. 3.2 should allow both, Patches on #17174 will probably be used to close this ticket.

Reclosing as fixed for 3.1.1.

See #17174 for 3.2