Make WordPress Core

Opened 15 years ago

Closed 14 years ago

#9269 closed defect (bug) (duplicate)

Proper handling of custom taxonomy in query

Reported by: dfellars1's profile dfellars1 Owned by: ryan's profile ryan
Milestone: Priority: normal
Severity: normal Version: 2.7.1
Component: Query Keywords: has-patch needs-testing
Focuses: Cc:

Description

Currently a custom taxonomy does not have the same behavior as tags/categories when dealing with permalink query. Getting a 404 with taxonomies when there are no posts associated with a taxonomy (I have hierarchical custom taxonomies, so some don't have posts)

There are 2 issues:

1) in classes.php handle_404() function, there is no exception check for is_tax() to override scenario if no posts associated with a custom tax - that should be a simple add of (is_tax()
is_tag().....

2) wp_queried_object() on this line in handle_404 returns NULL when there are no sub-terms associated with the term in the query. So it still process
This can get fixed either by removing the isempty($term) in wp_queried_object under this $this->is_tax block that causes it to return NULL in wp_queried_object. OR change the call to get_terms to add 'hide_empty' => false as an argument. Since in this scenario, it returns a single term where slug=$slug, but it gets removed since by default hide_empty is true and this term doesn't have any posts.

I've looked at who else would use this section for taxonomies and the only place I really saw it being used was in wp_head()

Attachments (3)

query.diff (144 bytes) - added by dfellars1 15 years ago.
add hide_empty=false to get_terms in get_queried_object
classes.diff (213 bytes) - added by dfellars1 15 years ago.
add is_tax() to handle_404()
9269.diff (1.2 KB) - added by Denis-de-Bernardy 15 years ago.
same patch, in diff format

Download all attachments as: .zip

Change History (21)

#1 in reply to: ↑ description @dfellars1
15 years ago

Just a side-not - a category will properly display the category/archive .php file even if no posts in that category, so I would hope that custom taxonomies work in similar fashion.

#2 @filosofo
15 years ago

You can query posts with a custom taxonomy: just create a custom rewrite rule and map to a custom query variable for that taxonomy.

In my opinion, the taxonomy structure is too abstract to assume that it is meant to be publicly accessible by default.

#3 follow-up: @dfellars1
15 years ago

Not sure I understand your comment. I don't think we are on the same page.

Some more clarification: I am using the 'custom taxonomy' plugin to handle any new custom taxonomies. This handles adding the custom rewrite rule for me (via add_permastruct) - so it is there. Everything works as expected if there is a post associated with a custom term taxonomy. Its just when there are no posts associated with that term where this issue introduces itself.

Wordpress already makes the taxonomy structure public by default - apparently just not that many people use it :)

I just want to fix the scenario for when a custom term taxonomy does not have any posts associated with it to not return a 404, but to go through proper template loading procedures (i.e. check for taxonomy-$taxonomy-$term.php, then taxonomy-$taxonomy.php, then taxonomy.php) instead of returning a 404 in this scenario. Especially since that is how categories work.

#4 in reply to: ↑ 3 @filosofo
15 years ago

Replying to dfellars1:

Wordpress already makes the taxonomy structure public by default - apparently just not that many people use it :)

Not really. It's your plugin's rewrite rule and consequent query that is making public the association between posts and a custom taxonomy.

I just want to fix the scenario for when a custom term taxonomy does not have any posts associated with it to not return a 404, but to go through proper template loading procedures (i.e. check for taxonomy-$taxonomy-$term.php, then taxonomy-$taxonomy.php, then taxonomy.php) instead of returning a 404 in this scenario. Especially since that is how categories work.

The problem is that putting something like this in core would be pointless without having the prerequisite association between a requested URL and query arguments: to load something like taxonomy-* the template loader would first need to know that a taxonomy is being requested. You're saying you want the cart, and I'm saying we shouldn't even have the horse. I would argue against default rewrite rules / query arguments for custom taxonomies for several reasons:

  • We don't necessarily want to expose custom taxonomies to public queries.
  • Custom taxonomies are often associated with non-post objects.
  • Excess custom taxonomy rewrite rules could present performance problems.
  • The current steps necessary to create rewrite rules and query variables necessary to query for taxonomies are so minimal, and need for this kind of behavior so obscure, that it doesn't warrant the problems that would arise by putting it in core.

#5 @dfellars1
15 years ago

let me backtrack a bit :)

By default I meant that it allows you to add the rewrite rules by default to make taxonomies public. Im not proposing to make that association by default in core - I want to leave that piece as is. By using the custom taxonomies plugin, it properly creates the rewrite rules for me and I agree that that should be done at the plugin level and not added to core. I apologize for confusion.

What I am saying, is that once the association has been made by the plugin and the rewrite rule has been added, I want it to work properly. And I don't believe it is under the scenario when there are no posts associated with a term/taxonomy. I believe if we are adding the association to the rewrite rules to tell it to properly deal with my taxonomies, that I should be able to display a custom taxonomy page when listing an empty taxonomy archive page - instead of getting a 404.

#6 @westi
15 years ago

  • Cc westi added
  • Keywords needs-patch added; taxonomy wp_queried_object removed
  • Milestone changed from Unassigned to 2.8

Could you try and write a patch to show your suggested solution.

I guess the goal is to allow for a plugin which creates a custom taxonomy to have the ability to have the empty query which current generates a 404 generate different output while still preserving the 404 default if the plugin doesn't so anything.

Tentatively scheduling for 2.8 but this won't happen unless a patch is written.

#7 @dfellars1
15 years ago

Did I do the diff files wrong - I just copied the functions where changes were made into their own file and uploaded. There were only 2 small changes:

classes.php: + is_tax()

query.php: + , 'hide_empty'=>false

first timer, sorry :)

@dfellars1
15 years ago

add hide_empty=false to get_terms in get_queried_object

@dfellars1
15 years ago

add is_tax() to handle_404()

#8 @dfellars1
15 years ago

  • Keywords has-patch added; needs-patch removed

#9 @Denis-de-Bernardy
15 years ago

patch is a bit broken:

DB:~/Sites/wp $ wptest http://core.trac.wordpress.org/attachment/ticket/9269/classes.diff
can't find file to patch at input line 1
Perhaps you used the wrong -p or --strip option?
File to patch: wp-includes/classes.php
patching file wp-includes/classes.php
Hunk #1 succeeded at 464 (offset 13 lines).
DB:~/Sites/wp $ svn diff
svn: File 'wp-includes/classes.php' has inconsistent newlines
svn: Inconsistent line ending style

@Denis-de-Bernardy
15 years ago

same patch, in diff format

#10 @Denis-de-Bernardy
15 years ago

  • Keywords needs-testing added

#11 @Denis-de-Bernardy
15 years ago

  • Keywords reporter-feedback added
  • Milestone changed from 2.8 to Future Release

@dfellars1: so, how does one test this?

Please bump this back to 2.8 (or 2.9 if it's released by then) with a test procedure, else you can be quite certain it'll get ignored, pending tests.

#13 @Denis-de-Bernardy
15 years ago

  • Component changed from Permalinks to Query
  • Milestone changed from Future Release to 2.9

#14 @ryan
14 years ago

  • Milestone changed from 2.9 to 3.0

#15 @Denis-de-Bernardy
14 years ago

  • Keywords reporter-feedback removed

#16 @hakre
14 years ago

Patch still applies clean.

A custom taxonomy related ticket is: #10207

#17 @hakre
14 years ago

Another custom taxnonomy related ticket is: #11612

#18 @dd32
14 years ago

  • Milestone 3.0 deleted
  • Resolution set to duplicate
  • Status changed from new to closed

I believe both of the issues outlined here have been fixed, the latter in #12533

Note: See TracTickets for help on using tickets.