post_type_archive_title doesn't work when tax_query is added to wp_query

[wonderboymusic]

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

[scribu]

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.

As a workaround for your problem, 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' );

[wonderboymusic]

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

[wonderboymusic]

Revised Patch for this ticket

[scribu]

Ok, and what happens if 'post_type' is set to an array by some other code that alters $wp_query?

[scribu]

PS: When you upload a revised patch, don't overwrite the previous version.

[wonderboymusic]

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"
}

[wonderboymusic]

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

[scribu]

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 is to be used only within The Loop.

[wonderboymusic]

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

[scribu]

Fair enough. Will let someone else weigh in.

[dd32]

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.

[johnbillion]

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.

[nacin]

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.

[SergeyBiryukov]

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.

[billerickson]

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

[nacin]

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.

[scribu]

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.

[SergeyBiryukov]

Related: #17662, #20519

[SergeyBiryukov]

Closed #20994 as a duplicate.

[SergeyBiryukov]

Related: #21394

[styledev]

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.

[SergeyBiryukov]

Replying to styledev:

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() )

Related: [21984], [22483] (for #21648).

[benjmay]

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

[markoheijnen]

Ben, I guess the notices weren't the same and for your notices there is a patch for that in ticket #21394

[wonderboymusic]

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

[styledev]

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

[wonderboymusic]

I forgot I even filed this ticket - @nacin patched this properly a long time ago, it was just never committed. I have refreshed against trunk, the scenario described in the ticket is still valid.

[wonderboymusic]

In 25291:

Ensure that the post type object is the queried object when a post type has been registered with has_archive => true. Ensure it is not stomped when decorated with tax_query. Adds unit tests.

Props nacin.
Fixes #18614.

[nacin]

1. get_post_type_object( get_query_var( 'post_type' ) ); — could that fail if the post_type QV is an array? That's the only thing that jumps out.

2. Could we get away with demoting the is_post_type_archive() and is_tax() conditionals in template-loader.php to below is_front_page() and is_home()? Then is_home() will happen right before is_post_type_archive(), possibly setting the stage for that to be cleaned up in the future, as it is essentially the 'post' post type archive (plus some other special sauce). It would also make a lot more sense as is_tax() was kind of nonchalantly stuffed up at the top originally. Does anyone see any side effects?

[wonderboymusic]

1. This check needs to happen in get_post_type_object(). If an array is passed, in needs to do $post_type = reset( $post_type );. There are 8-10 places in core where the query var is passed.

2. As long as is_tax() is false when the post_type obj is the queried object, I think it is safe.

[wonderboymusic]

In 25292:

Check the value passed to get_post_type_object(). If it's an array, use the first item. get_query_var( 'post_type' ) can be an array if the query has been altered via filters/actions. There are several places in core that pass the query var. Adds unit tests.

In template-loader.php, move is_post_type_archive() and is_tax() directly below is_home().

See #18614, [25291].

[nacin]

Sorry to be a pain, but I feel like the reset() in [25292] might not be the best place for this. Or at the very least, it needs docs.

If you are passing an array to get_post_type_object(), you *may* be doing it wrong. For us to choose the first one (essentially picking favorites) could mean we are suppressing issues. While it might make sense in some places to choose the first one, it might not make sense in all places. This could potentially paper over a developer error. (Including, yes, an error where we have done this 8-10 times ourselves.)

I was only musing for the template-loader.php change. Let's dig into that a bit more to make sure it won't break anything. Also, [25292] should have been two commits.

[wonderboymusic]

yeah, I was a shade overeager - I'll inspect those places in core and see if the checks make more sense inline

[nacin]

By wonderboymusic, in [25312]: Move checks for post_type being an array inline. See [25291], [25292], #18614.

(The ticket(s) has to come before any changeset references for "see" or "fixes" to work.)

[nacin]

And regarding [25312], even better, some of these aren't even needed inline, because is_post_type_archive is only true when post_type is *not* an array:

if ( !empty( $qv['post_type'] ) && ! is_array( $qv['post_type'] ) ) {
	$post_type_obj = get_post_type_object( $qv['post_type'] );
	if ( ! empty( $post_type_obj->has_archive ) )
		$this->is_post_type_archive = true;
}

[nacin]

See #25341 for problems.

[nacin]

This ticket still has a needs-unit-tests keyword, but seems like everything is good for 3.7.