Opened 21 months ago
Last modified 7 weeks ago
#18614 reopened defect (bug)
post_type_archive_title doesn't work when tax_query is added to wp_query
| Reported by: |
|
Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | 3.6 |
| Component: | Template | Version: | 3.2 |
| Severity: | normal | Keywords: | has-patch needs-refresh |
| Cc: | bill.erickson@…, styledev, benjmay |
Description
post_type_archive_title( ) gets called by wp_title( ) on Custom Post Type archive pages
To get the data for the post type object, post_type_archive_title( ) calls get_queried_object( ). This is problematic. If the query has been altered in any way (such as adding a tax_query in pre_get_posts filter), get_queried_object( ) will probably return a term, not a post type object.
Hence, accessing $post_type_obj->labels will fail and their will be no title returned
//old potentially broken way $post_type_obj = get_queried_object(); // new doesn't break way $post_type_obj = get_post_type_object( get_post_type() );
Attachments (3)
Change History (29)
- Milestone Awaiting Review deleted
- Resolution set to wontfix
- Status changed from new to closed
comment:2
wonderboymusic — 21 months ago
post_type_archive_title( ) exits if the current context is not post_type_archive( ) - meaning post_type is set in $wp_query.
The name of the variable is $post_type_obj which leads me to believe it should always be a post type object, which get_queried_object isn't guaranteed to return. Since post_type is in wp_query, get_post_type will work and get_post_type_object( get_post_type() ) will work and return a post_type_object, so then $post_type_obj->labels won't fail and log errors
This isn't a band aid, writing a filter to correct what should be default functionality is
Ok, and what happens if 'post_type' is set to an array by some other code that alters $wp_query?
PS: When you upload a revised patch, don't overwrite the previous version.
comment:5
in reply to:
↑ 3
;
follow-up:
↓ 6
wonderboymusic — 21 months ago
Replying to scribu:
Ok, and what happens if 'post_type' is set to an array by some other code that alters $wp_query?
funny you should ask - I did some testing that looks at exactly that - removes the tax_query and instead sets post_type (also tested with tax_query AND post_type set to something custom)
the original post_type was "emusic_icon" and I added "post" and "emusic_spotlight"
// altered function
function post_type_archive_title( $prefix = '', $display = true ) {
global $wp_query;
var_dump( $wp_query );
var_dump( get_post_type_object( get_post_type() ) ); exit();
if ( ! is_post_type_archive() )
return;
$post_type_obj = get_post_type_object( get_post_type() );
$title = apply_filters('post_type_archive_title', $post_type_obj->labels->name );
if ( $display )
echo $prefix . $title;
else
return $title;
}
Here's the output for $wp_query with an Array of post_types:
object(WP_Query)#129 (47) {
["query_vars"]=>
array(58) {
["post_type"]=>
array(3) {
[0]=>
string(4) "post"
[1]=>
string(11) "emusic_icon"
[2]=>
string(16) "emusic_spotlight"
}
.....
Here's the output for get_post_type_object( get_post_type() ) - returns the ORIGINAL post_type:
object(stdClass)#354 (26) {
["labels"]=>
object(stdClass)#356 (14) {
["name"]=>
string(5) "Icons"
["singular_name"]=>
string(4) "Icon"
["add_new"]=>
string(7) "Add New"
["add_new_item"]=>
string(12) "Add New Icon"
["edit_item"]=>
string(9) "Edit Icon"
["new_item"]=>
string(8) "New Icon"
["view_item"]=>
string(9) "View Icon"
["search_items"]=>
string(12) "Search Icons"
["not_found"]=>
string(14) "No Icons found"
["not_found_in_trash"]=>
string(23) "No Icons found in Trash"
["parent_item_colon"]=>
NULL
["all_items"]=>
string(5) "Icons"
["menu_name"]=>
string(5) "Icons"
["name_admin_bar"]=>
string(4) "Icon"
}
["description"]=>
string(0) ""
["publicly_queryable"]=>
bool(true)
["exclude_from_search"]=>
bool(false)
["capability_type"]=>
string(4) "post"
["map_meta_cap"]=>
bool(true)
["_builtin"]=>
bool(false)
["_edit_link"]=>
string(16) "post.php?post=%d"
["hierarchical"]=>
bool(false)
["public"]=>
bool(true)
["rewrite"]=>
array(4) {
["slug"]=>
string(15) "music-news/icon"
["with_front"]=>
bool(true)
["pages"]=>
bool(true)
["feeds"]=>
bool(true)
}
["has_archive"]=>
string(16) "music-news/icons"
["query_var"]=>
string(11) "emusic_icon"
["register_meta_box_cb"]=>
object(Closure)#353 (1) {
["static"]=>
array(2) {
["type"]=>
string(11) "emusic_icon"
["callback"]=>
object(Closure)#352 (1) {
["parameter"]=>
array(1) {
["$type"]=>
string(10) "<required>"
}
}
}
}
["taxonomies"]=>
array(5) {
[0]=>
string(6) "region"
[1]=>
string(5) "genre"
[2]=>
string(8) "location"
[3]=>
string(8) "post_tag"
[4]=>
string(8) "category"
}
["show_ui"]=>
bool(true)
["menu_position"]=>
NULL
["menu_icon"]=>
NULL
["permalink_epmask"]=>
int(1)
["can_export"]=>
bool(true)
["show_in_nav_menus"]=>
bool(true)
["show_in_menu"]=>
bool(false)
["show_in_admin_bar"]=>
bool(false)
["name"]=>
string(11) "emusic_icon"
["cap"]=>
object(stdClass)#355 (14) {
["edit_post"]=>
string(9) "edit_post"
["read_post"]=>
string(9) "read_post"
["delete_post"]=>
string(11) "delete_post"
["edit_posts"]=>
string(10) "edit_posts"
["edit_others_posts"]=>
string(17) "edit_others_posts"
["publish_posts"]=>
string(13) "publish_posts"
["read_private_posts"]=>
string(18) "read_private_posts"
["read"]=>
string(4) "read"
["delete_posts"]=>
string(12) "delete_posts"
["delete_private_posts"]=>
string(20) "delete_private_posts"
["delete_published_posts"]=>
string(22) "delete_published_posts"
["delete_others_posts"]=>
string(19) "delete_others_posts"
["edit_private_posts"]=>
string(18) "edit_private_posts"
["edit_published_posts"]=>
string(20) "edit_published_posts"
}
["label"]=>
string(5) "Icons"
}
comment:6
in reply to:
↑ 5
wonderboymusic — 21 months ago
I think the above behavior is correct - not to mention that it returns a post_type label name, instead of whatever taxonomy label name for whatever term that get_queried_object returns
Yes, but do you know why it works?
Because get_post_type() checks the $post global:
http://core.trac.wordpress.org/browser/trunk/wp-includes/post.php#L786
In other words, it's is to be used only within The Loop.
comment:8
wonderboymusic — 21 months ago
ok - then this is still better than returning a term and trying to access a property that doesn't exist:
$type = get_query_var( 'post_type' ) ; $post_type_obj = get_post_type_object( is_array( $type ) ? $type[0] : $type );
- Keywords 2nd-opinion added
- Milestone set to Awaiting Review
- Resolution wontfix deleted
- Status changed from closed to reopened
Fair enough. Will let someone else weigh in.
comment:10
dd32 — 21 months ago
Just to chime in here, This is something I've run into a few times - not this specifically, but relying upon get_queried_object() to return the same data as expected. If it's an "advanced Query" (ie. Taxonomy(s) or Authors set - any type of archive I believe) then the return may not be exactly what the calling code is expecting.
comment:11
johnbillion — 19 months ago
It's worth noting that this is reproducable using public query vars (eg. example.com/my_post_type/?my_taxonomy=my_term) as reported in #19035.
comment:12
nacin — 18 months ago
- Version changed from 3.3 to 3.2
So is_post_type_archive() returns true but the queried object is wrong? I'm fine with the patch, but this seems like it's a larger issue.
From #19035:
Example: When viewing example.com/my_post_type/?my_taxonomy=my_term, the return value of get_queried_object() is the term object, not the post type object. This causes a PHP notice when post_type_archive_title() attempts to access the label for the post type object, and results in a blank title.
comment:14
billerickson — 18 months ago
- Cc bill.erickson@… added
+1 for the patch.
I just ran into this issue on taxonomy archives limited to a specific post type ( example.com/taxonomy/term/?post_type=my_post_type ).
I'm using the filter to rebuild the title and just ignoring the PHP notice for now.
comment:15
nacin — 15 months ago
18614.diff is the first pass at different route to solve the larger problem here, which includes #19939 as well as this ticket.
18614.2.diff is a second take, which, primarily, does not re-order anything in get_queried_object(). What both patches do make sure of, though, is that is_post_type_archive() is always requested prior to taxonomy data.
This was a bit inconsistent in the original implementation. wp_title() is going to be correct since it will always allow a post type archive to override a taxonomy. Given the hierarchy inherent to posts and terms, I would think that a post type archive should nearly always take precedence. While it is nearly impossible to tell the difference between /tag/some-tag/?post_type=type and /type/?tag=some-tag (and certainly so when rewrite rules are off), I think a good assumption would be that if the post type is has_archive = true, it is going to be the main target of a drill-down, rather than an option of a drill-down. (Or, at least, it is a higher priority than a tag or category in the subject of a drill-down, generally speaking.)
I thought about get_queried_object( $type_of_object ), get_queried_objects(), and a few other things here. I think greater changes should wait for another day. For now, the attached patch attempts to bring post types above taxonomies when has_archive is true, and solve the current issue of template tags malfunctioning.
comment:16
scribu — 15 months ago
- Keywords 2nd-opinion removed
- Milestone changed from Awaiting Review to 3.4
I haven't tested either of the patches, but I agree with the reasoning:
If we start with the premise that post types should never be mixed between them, then the post type should indeed take precedence over taxonomies, which can accept multiple terms.
comment:18
ryan — 13 months ago
- Milestone changed from 3.4 to Future Release
Closed #20994 as a duplicate.
comment:20
SergeyBiryukov — 9 months ago
Related: #21394
comment:21
follow-up:
↓ 22
styledev — 5 months ago
- Cc styledev added
It seems as if a part of 18614.2.diff was included in Wordpress 3.5 in general-template.php lines 1666 to 1669 in function feed_links_extra( $args = array() ):
} elseif ( is_post_type_archive() ) {
$title = sprintf( $args['posttypetitle'], get_bloginfo('name'), $args['separator'], post_type_archive_title( '', false ) );
$href = get_post_type_archive_feed_link( get_queried_object()->name );
}
Which now is causing the following notices:
Notice: Undefined property: stdClass::$labels in /Users/dbushaw/Sites/trainelite/wp-includes/general-template.php on line 658
Notice: Trying to get property of non-object in /Users/dbushaw/Sites/trainelite/wp-includes/general-template.php on line 658
Note: If you apply the latest patch (18614.2.diff) then everything is fixed.
comment:22
in reply to:
↑ 21
SergeyBiryukov — 5 months ago
comment:23
benjmay — 5 months ago
- Cc benjmay added
I was getting the same notices as @styledev, however, it wasn't from wp_title as I was being clever and only using wp_title on non-CPT pages.
The error was coming from the 'extra' feeds in wp_header.
I was able to prevent the error without making mods to core by simply adding:
remove_action( 'wp_head', 'feed_links_extra', 3 );
comment:24
markoheijnen — 5 months ago
Ben, I guess the notices weren't the same and for your notices there is a patch for that in ticket #21394
comment:25
wonderboymusic — 5 months ago
- Keywords needs-refresh added
- Milestone changed from Future Release to 3.6
While it is nearly impossible to tell the difference between /tag/some-tag/?post_type=type and /type/?tag=some-tag (and certainly so when rewrite rules are off)
WP::$matched_query is always correct when it comes to precedence when rewrite rules are on.
When the URL is wordpress-core/tag/woo/?post_type=sport:
["matched_query"]=> string(7) "tag=woo"
When the URL is wordpress-core/sport/?tag=woo:
["matched_query"]=> string(15) "post_type=sport"
Relying on the WP property will also prevent a tax_query from stomping the matched_query in get_queried_object
comment:26
styledev — 7 weeks ago
Since this is taking a while to get fixed here is a function you can put in your functions.php file to turn of the is_tax on these types of queries. https://gist.github.com/styledev/5278527

What about when you are on a taxonomy page and use 'pre_get_posts' to set an author etc. etc.? We can't apply band-aids like this.
You can reset $wp_query->queried_object to whatever you need, without modifying Core:
function change_queried_object() { global $wp_query; if ( is_post_type_archive() ) $wp_query->queried_object = get_post_type_object( get_post_type() ); } add_action( 'template_redirect', 'change_queried_object' );