get_post_class() does not always apply filter to output

Description

Currently the post class function/template tag simply returns an empty array for 404 requests. The reason is simple: The output gets generated based on the return values of get_post().

This avoids using it to style parts of the 404 template using the post_class filter.

Use case: In one theme I'm currently developing, I use the post_class filter to override the parent themes classes with some grid classes from a CSS framework. This avoids duplicating the templates and handle the positions, etc. of the template parts with a simple callback for the post_class filter. The attached patch makes sure that classes get added, instead of just returning an empty array if no $post->ID is present. It doesn't harm any other callback (or interfere with it).

The attached patch is tested in current 3.4.2 and 3.5 Beta. It is built against the v3.5 nightly build - revision 606806.

It seems to me that you're trying to transform get_post_class() into something that dilutes it's purpose, just because it's convenient for you, instead of fixing your theme to account for the is_404() case.

It seems to me that you're trying to transform get_post_class() into something that dilutes it's purpose, just because it's convenient for you, instead of fixing your theme to account for the is_404() case.

Making sure it always runs the filter is "diluting it's purpose?" Sounds like avoidance of change for avoidance of change sake.

"You're avoiding change for the sake of avoiding change" is something you can throw around whenever you disagree with a wontfix, regardless of what argument was made.

It detracts from the actual discussion, steering it into meta discussion about governance.

And I replied as I did because I was reacting to how frequently you reply in a manner that is disrespectful to others in the community who are trying to contribute. Had you simply held your comment to be "Yes, it does: filters that don't expect $post to be null will break" and not added ​the disrespectful comment I wouldn't have replied as I did. But yes, this is a meta/governance issue so that's all I'll say on this ticket.

I apologize if my tone came off as disrespectful, but I did have a point. I'm guilty of it myself: trying to push a change into Core that would be helpful for the narrow problem I'm trying to solve at the time, not realizing that it's actually a bad idea in the general case.

This is generic expectation. I expect (possibly mistakenly, but it makes sense to me) WP to behave like this and would be (am) surprised when it's not the case. Note that "why would it apply filter in some specific case" defeats this expectation in generic form. It is "filter return", not "filter return sometimes".

post_class filter will always pass valid post ID.

This is specific expectation. It is formed by how code works historically and in my perception was not intentionally engineered. It is not a goal of this filter to only pass valid ID. ID is just meta information there without explicit guarantee of validity. There are a lot of functions that verify if they have been passed valid input (including get_post_class() itself) instead of arguing that they should always receive only valid input.

So what this ticket boils down - which of the two expectations is more important to uphold.

WordPress often applies a filter to all return values, but sometimes will not do so in an error, or will use an alternate filter name for unexpected or sudden returns. This seems like a good situation for there to be a unique filter.

Those applying a post class will be doing so — in nearly all cases — with the expectation that $post is indeed an object. This seems like a very reasonable expectation, and practically, things could break if we try to apply the same post_class filter in this case.

Apply filter in all cases (breaks with Notice when $post object is expected and debug turned on)

Apply special-ops filter (needs new patch)

Leave current state (close ticket)

Other possibilities how to resolve this:

Return just the classes the user added as argument to post_class( 'example-class' );. This is a minimum "solution". It helps working around those cases where the index.php triggers as fallback for non existing 404.php templates and no check for have_posts() exists (but just a HTML wrapper tag that has post_class(); attached). (needs new patch)

Leave everything as it is and introduce a new template_part_class(); (or however this could be named) template tag that should be used on wrappers. Then just add a _doing_it_wrong(); if post_class(); is ! in_the_loop(); to deny further "wrong" usage. (needs new patch + new ticket)

get_post_class() does not pass post object, only ID. Any filter will have to retrieve post object anyway... and as for me it's filter's problem if it doesn't check that if that came up with something.

there are other filters that will happily pass non-valid ID, for example get_the_title. So case can be made that specific get_post_class filter historically only passes valid ID, but there is no general case that filter passing ID as additional argument must ensure that it is valid one.

PS introducing new filter doesn't make sense to me because I don't see strong case to not just apply existing to all cases and be done.

get_post_class() does not pass post object, only ID. Any filter will have to retrieve post object anyway... and as for me it's filter's problem if it doesn't check that if that came up with something.

there are other filters that will happily pass non-valid ID, for example get_the_title. So case can be made that specific get_post_class filter historically only passes valid ID, but there is no general case that filter passing ID as additional argument must ensure that it is valid one.

I took a look at other functions like the_content();, the_guid(), etc. There're dozens that behave like this. This kept me thinking about it for a while.

get_post_class() is a public function. It neither has leading _/underscore to tell that it's not meant for public usage, nor a line stating this in the phpDocBlock.

When we then take a look at the input arguments, then both $classes as well as $post_id are optional.

So the default behavior of this function is to simply fail, returning the empty array(), when publicly used and no global $post object is present that can be used as fallback inside get_post(). And that is strange.

The error that happens if one doesn't check if an object is set, will only be visible with WP_DEBUG (or PHP error env) turned on. And that shouldn't happen in production.

Based on all heard ideas and arguments so far, my personal preference would be to do the merge as early it is in scribus patch (no need to merge with the final array - speeds things up slightly), but also move the filter in for early returns.

I think back compat trumps the expectation that a filter will be applied to all return values. As nacin pointed out, WordPress is pretty consistent about that inconsistency, and in the context of get_post_class() I'd consider the lack of post context at least "unexpected", if not an "error".