[wp-trac] [WordPress Trac] #33728: Rewrite endpoints cannot be added to custom taxonomies with public rewrites

WordPress Trac noreply at wordpress.org
Sun Sep 6 00:35:44 UTC 2015


#33728: Rewrite endpoints cannot be added to custom taxonomies with public rewrites
------------------------------------------+-----------------------------
 Reporter:  mordauk                       |       Owner:
     Type:  defect (bug)                  |      Status:  new
 Priority:  normal                        |   Milestone:  Future Release
Component:  Rewrite Rules                 |     Version:  trunk
 Severity:  normal                        |  Resolution:
 Keywords:  needs-patch needs-unit-tests  |     Focuses:
------------------------------------------+-----------------------------
Changes (by johnbillion):

 * keywords:   => needs-patch needs-unit-tests
 * milestone:  Awaiting Review => Future Release


Old description:

> 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 );
> }}}
>
>  2. 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`
>
> 2. 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.
>
> 2. 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](https://core.trac.wordpress.org/ticket/21050), and that
> `EP_TAXONOMY` be included in `EP_ALL`.
>
> Related [19275](https://core.trac.wordpress.org/ticket/19275)

New description:

 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 );
 }}}

  2. 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`

 2. 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.

 2. 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

--

Comment:

 I concur.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/33728#comment:2>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform


More information about the wp-trac mailing list