WordPress.org

Make WordPress Core

Opened 6 years ago

Last modified 4 years ago

#33728 reviewing defect (bug)

Rewrite endpoints cannot be added to custom taxonomies with public rewrites — at Version 2

Reported by: mordauk Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 4.4
Component: Rewrite Rules Keywords: needs-unit-tests has-patch needs-refresh
Focuses: Cc:

Description (last modified by johnbillion)

Take the following scenario:

  1. A plugin registers a custom taxonomy that is public and has pretty rewrite rules enabled:
function genre_taxonomy() {

	$tag_args = array(
		'label'   => 'Genre',
		'public'  => true,
		'rewrite' => array( 'slug' => 'genre' ),
	);
	register_taxonomy( 'genre', array( 'post' ), $tag_args );

}
add_action( 'init', 'genre_taxonomy', 5 );
  1. A second plugin registers a rewrite endpoint that is added to all URLs on the site:
function ajax_endpoint() {

	add_rewrite_endpoint( 'ajax', EP_ALL );

}
add_action( 'init', 'ajax_endpoint' );

EP_ALL means "add this to all pages", which should logically include all custom taxonomies.

This however, is not the case.

Works:

  • site.com/ajax/1
  • site.com/tag/blug/ajax/1
  • site.com/blog/hello-world/ajax/1
  • site.com/2015/05/01/ajax/1
  • all other standard rewrites

Fails:

  • site.com/genre/rock/ajax/1

It fails because custom taxonomies do not get included in EP_ALL unless they are explicitly included. If we look at register_taxonomy(), we see this is done intentionally:

	if ( false !== $args['rewrite'] && ( is_admin() || '' != get_option( 'permalink_structure' ) ) ) {
		$args['rewrite'] = wp_parse_args( $args['rewrite'], array(
			'with_front' => true,
			'hierarchical' => false,
			'ep_mask' => EP_NONE,
		) );

		if ( empty( $args['rewrite']['slug'] ) )
			$args['rewrite']['slug'] = sanitize_title_with_dashes( $taxonomy );

		if ( $args['hierarchical'] && $args['rewrite']['hierarchical'] )
			$tag = '(.+?)';
		else
			$tag = '([^/]+)';

		add_rewrite_tag( "%$taxonomy%", $tag, $args['query_var'] ? "{$args['query_var']}=" : "taxonomy=$taxonomy&term=" );
		add_permastruct( $taxonomy, "{$args['rewrite']['slug']}/%$taxonomy%", $args['rewrite'] );
	}

In my opinion, this is fundamentally wrong. If a rewrite endpoint is added with EP_ALL, it should actually be registerred on all rewrites, not just some.

Let's see another example to help illustrate that this is wrong.

  1. A plugin (such as WooCommerce) registers a custom taxonomy with public rewrites called "Product Category". With this taxonomy, the following rewrite is available: site.com/products/product-category/mugs
  1. A second plugin (such as AffiliateWP) registers a rewrite endpoint with EP_ALL to permit the following:
  • site.com/ref/1
  • site.com/page-name/ref/1
  • site.com/2015/01/01/ref/1
  • site.com/category/news/ref/1
  • etc, etc
  • AND
  • site.com/products/product-name/ref/1 (works)
  • site.com/products/product-category/mugs/ref/1 (fails)

The rewrite endpoint works fine on custom post type rewrites but not taxonomy rewrites.

There are two ways for rewrite endpoints to work on the custom taxonomy:

  1. Have the taxonomy include 'ep_mask' => 'EP_ALL' (or similar) when it is registered. This requires that the plugin that registers the taxonomy to include this, which very, very, very few do as it is not necessary for pretty permalinks to work.
  1. Manually add rewrite rules for the endpoints to all custom taxonomies:
$taxonomies = get_taxonomies( array( 'public' => true, '_builtin' => false ), 'objects' );

foreach( $taxonomies as $tax_id => $tax ) {
	add_rewrite_rule( $tax->rewrite['slug'] . '/(.+?)/ref(/(.*))?/?$', 'index.php?' . $tax_id . '=$matches[1]&ref=$matches[3]', 'top');		
	
}

While manually adding the rewrite rules does work, it should not be necessary when add_rewrite_endpoint( 'ref', EP_ALL ); should be sufficient.

I propose that EP_TAXONOMY be introduced, as suggested in #21050, and that EP_TAXONOMY be included in EP_ALL.

Related #19275

Change History (3)

#1 @mordauk
6 years ago

Another example to illustrate that this is incorrect.

If a custom taxonomy is registered with 'rewrite' => false, the rewrite rule created by add_rewrite_endpoint() does get registered for the taxonomy, which is rather illogical since the taxonomy doesn't even support rewrites.

#2 @johnbillion
6 years ago

  • Description modified (diff)
  • Keywords needs-patch needs-unit-tests added
  • Milestone changed from Awaiting Review to Future Release

I concur.

@mordauk
6 years ago

Note: See TracTickets for help on using tickets.