Make WordPress Core

Opened 14 years ago

Closed 13 years ago

Last modified 13 years ago

#12659 closed enhancement (fixed)

Hierarchical Taxonomy URL's do not include parent terms

Reported by: dd32's profile dd32 Owned by: scribu's profile 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 13 years ago.
Initial scratch-run at it.

Download all attachments as: .zip

Change History (38)

#1 @scribu
14 years ago

  • Cc scribu@… added

#2 @jfarthing84
14 years ago

  • Cc jeff@… added

I was just about to report this.

#3 @filosofo
14 years ago

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

#4 @kevinB
14 years ago

  • Cc kevinB added

#5 @litemotiv
14 years ago

  • Cc litemotiv added

#6 @aaroncampbell
14 years ago

  • Cc aaroncampbell added

#7 @scribu
14 years ago

Related: #12891

#8 @epicalex
14 years ago

  • Cc epicalex added

#9 follow-up: @dd32
13 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
13 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
13 years ago

  • Milestone changed from Awaiting Triage to 3.1

@dd32
13 years ago

Initial scratch-run at it.

#12 @dd32
13 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
13 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
13 years ago

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

#15 @dd32
13 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
13 years ago

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

#17 @nacin
13 years ago

Closed #13889 as a duplicate.

Shall we move hierarchical_url() under rewrite?

#18 @dd32
13 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
13 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
13 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
13 years ago

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

#22 @scribu
13 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
13 years ago

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

#24 @scribu
13 years ago

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

#25 @dd32
13 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
13 years ago

I guess you're right.

#27 @jane
13 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
13 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
13 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
13 years ago

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

#31 @danieliser
13 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
13 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
13 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
13 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
13 years ago

This appears to have broken PATHINFO taxonomy permalinks:

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

#36 @markjaquith
13 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
13 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.