WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 5 years ago

#12659 closed enhancement (fixed)

Hierarchical Taxonomy URL's do not include parent terms

Reported by: dd32 Owned by: scribu
Milestone: 3.1 Priority: normal
Severity: normal Version: 3.0
Component: Taxonomy Keywords: needs-patch
Focuses: Cc:

Description

I just noticed that for all custom taxonomies, the url format is /base/slug/ - the same as tags.

Surely for hierarchical taxonomies, it makes more sense to do /base/parentslug/childslug/ ? Much like categories.

Setting to 3.1 milestone as its not a regression from a previous release. With the hierarchical taxonomies exposed in 3.0 however, this could become a quickly asked question why it doesnt operate like categories.

Attachments (1)

12659.diff (4.6 KB) - added by dd32 6 years ago.
Initial scratch-run at it.

Download all attachments as: .zip

Change History (38)

#1 @scribu
6 years ago

  • Cc scribu@… added

#2 @jfarthing84
6 years ago

  • Cc jeff@… added

I was just about to report this.

#3 @filosofo
6 years ago

  • Owner changed from filosofo to dd32
  • Status changed from new to assigned

#4 @kevinB
6 years ago

  • Cc kevinB added

#5 @litemotiv
6 years ago

  • Cc litemotiv added

#6 @aaroncampbell
6 years ago

  • Cc aaroncampbell added

#7 @scribu
6 years ago

Related: #12891

#8 @epicalex
6 years ago

  • Cc epicalex added

#9 follow-up: @dd32
6 years ago

For back-compat issues, I'm thinking that this should be turned on by a specific flag in register_taxonomy(), 'hierarchical_uri' or similar?

Some of us have already implemented this using custom code, as well as there being links out there at present which would cause canonical redirections if it was made default.

#10 in reply to: ↑ 9 @nacin
6 years ago

Replying to dd32:

For back-compat issues, I'm thinking that this should be turned on by a specific flag in register_taxonomy(), 'hierarchical_uri' or similar?

Makes sense to me.

#11 @nacin
6 years ago

  • Milestone changed from Awaiting Triage to 3.1

@dd32
6 years ago

Initial scratch-run at it.

#12 @dd32
6 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

attachment 12659.diff added

  • Initial run
  • Non-optimized
    • Causes multiple repeditive queries (May just be showing that Taxonomy-related queries are not always cached)
  • Implements Canonicalisation as well

In order to test this, you'll need to register a taxonomy as such, AND flush your rewrite rules:

	register_taxonomy('state', array('post'), array('hierarchical' => true, 'hierarchical_uri' => true, 'label' => 'States', 'singular_label' => 'State', 'show_ui' => true, 'query_var' => 'state', 'rewrite' => true) );
	global $wp_rewrite;
	$wp_rewrite->flush_rules();

URL previously: http://localhost/wordpress-commit/state/kyogle/ After: http://localhost/wordpress-commit/state/au/nsw/kyogle/

#13 @dd32
6 years ago

(In [15705]) Introduce hierarchical taxonomy URL's, Can be enabled by setting 'hierarchical_url' to true upon taxonomy registration. See #12659

#14 @scribu
6 years ago

Wouldn't it be better if hierarchical_url was an option inside the 'rewrite' arg?

#15 @dd32
6 years ago

Wouldn't it be better if hierarchical_url was an option inside the 'rewrite' arg?

I like the way you think.

#16 @scribu
6 years ago

(In [15732]) Make hierarchical URLs work for any hierarchical taxonomy. See #12659

#17 @nacin
6 years ago

Closed #13889 as a duplicate.

Shall we move hierarchical_url() under rewrite?

#18 @dd32
6 years ago

Shall we move hierarchical_url() under rewrite?

It's coming, I've got a conflict between it and another tax rewrite item right now, I'll have some time this weekend to tidy it up

#19 @dd32
6 years ago

See [15824] Move hierarchical_url to $argsrewrite?hierarchical?

Note: This broke sub-category querying, I'm aware of this, I'm working on another patch to address rewrite items.

#20 @dd32
6 years ago

(In [15825]) Merge Category/Tag URL creation/rewriting into general Taxonomy system. Removes the legacy handling for these url's. See #12659

#21 @automattor
6 years ago

(In [15826]) Remove missed tokens in [15825]. See #12659

#22 @scribu
6 years ago

  • Keywords needs-patch added; has-patch needs-testing removed

[15825] introduces this notice when viewing a category page:

Notice: Undefined index: taxonomy in /home/cristi/svn/wp/wp-includes/query.php on line 1469:

	if ( $taxonomy == $q['taxonomy'] )
		$q['term'] = basename($q['term']);

I don't think those lines should be there at all.

Either we set 'taxonomy' and 'term' consistently, for all taxonomies, or we don't bother and encourage devs to use $wp_query->get_queried_object() to retrieve that information.

#23 @scribu
6 years ago

Actually, that will always be false, since it's in the else branch of !empty($q['taxonomy'])

#24 @scribu
6 years ago

(In [15921]) Remove misplaced code in WP_Query::parse_tax_query(). See #12659

#25 @dd32
6 years ago

[15825] introduces this notice when viewing a category page:

That code was there to prevent notices on custom taxonomy pages, Since it was inserted, It seems another commit has caused $q['taxonomy'] to be unset.

If code such as this is used on a generic template:

$link =  get_term_link(get_query_var('term'), get_query_var('taxonomy'));

that'll cause one or the other not to be set.

wp_title() also uses it:

	// If there's a taxonomy
	if ( is_tax() ) {
		$tax = get_taxonomy( get_query_var('taxonomy') );
		$title = single_term_title( $tax->labels->name . $t_sep, false );
	}

While it may not be the "best" way of getting the data, That's how it's currently done in many themes that I've seen. Supporting $term and $taxonomy for backwards compatibility has to happen, Even if it's only the first taxonomy/term which is being queried on a multi-taxonomy query.

#26 @scribu
6 years ago

I guess you're right.

#27 @jane
6 years ago

@dd32 is going to be on vacation for the next two weeks. Does someone else want to pick this up and get it finished before freeze so it makes 3.1?

#28 @dd32
6 years ago

The majority of this is already in, I'll leave the back-compat of the term/taxonomy query vars to Scribu however as he's done the majority of the work with the taxonomy query vars of late.

#29 @dd32
6 years ago

The majority of this is already in

Meaning, As long as no bugs crop up in testing, This should be good to go.

#30 @scribu
6 years ago

  • Owner changed from dd32 to scribu
  • Status changed from assigned to accepted

#31 @danieliser
6 years ago

I added these patches to a test site and they work for the taxonomies themselves but posts still have permalinks of post-type/post-name, instead of post-type/terms/post-name/.

Also noticed that part of this patch is in the Nightly build, but not only does it not work but part of the patch is missing.

#32 @dd32
6 years ago

I added these patches to a test site and they work for the taxonomies themselves but posts still have permalinks of post-type/post-name, instead of post-type/terms/post-name/.

That's the expected format. post-type url's are post_type/parent_post-name/post-name/ if set as hierarchical. This affects Taxonomy archives, such as /state/usa/california/ (Previously, would've just been /state/california/)

Also noticed that part of this patch is in the Nightly build, but not only does it not work but part of the patch is missing.

Feel like expanding on that a bit more? what's missing?

#33 @scribu
6 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

(In [16381]) Set 'taxonomy' and 'term' query vars for back-compat. Fixes #12659

#34 @dcowgill
6 years ago

Thanks for getting this fix in. I spent hours pulling my hair out thinking it was a param I was missing from register_taxonomy. Glad I'm not crazy. :-)

I've tested this on 3.1 beta and it works great.

'rewrite' => array( 'slug' => 'job-category', 'hierarchical' => true )


#35 @markjaquith
6 years ago

This appears to have broken PATHINFO taxonomy permalinks:

http://core.trac.wordpress.org/ticket/15813

#36 @markjaquith
5 years ago

(In [17512]) Prevent double index.php preprend on PATHINFO custom taxonomy permalinks. Proper use of with_front. props greuben. fixes #16918. fixes #16622. see #15813. see #12659. For trunk

#37 @markjaquith
5 years ago

(In [17513]) Prevent double index.php preprend on PATHINFO custom taxonomy permalinks. Proper use of with_front. props greuben. fixes #16918. fixes #16622. see #15813. see #12659. For 3.1

Note: See TracTickets for help on using tickets.