Make WordPress Core

Opened 3 years ago

Last modified 4 months ago

#22080 new enhancement

add_theme_support should merge post-thumbnail post types by default (currently stomps)

Reported by: alexkingorg Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 2.9
Component: Post Thumbnails Keywords: has-patch needs-refresh
Focuses: Cc:


As documented in the Codex:


you can pass a second param to add_theme_support( 'post-thumbnails' ) to specify the post types you want support enabled for. I expected any post types I passed in to be added to the currently specified post types, instead the passed value stomps the existing value. This means that if you create plugin that registers a post type and enables post-thumbnails for it, you need to write code like this:


instead of a simple:

add_theme_support( 'post-thumbnails', array( 'my-new-post-type' ) );

The latter makes more sense to me, and I think it's likely other devs might make the same mistake and accidentally stomp on the previous value for post-thumbnails.

The attached patch enables a merge or stomp behavior for post-thumbnails. It defaults to "merge". Passing a bool true as a 3rd param will cause the second param to replace the existing value (current behavior).

Attachments (3)

add-theme-support-without-stomp.diff (1.3 KB) - added by alexkingorg 3 years ago.
add-remove-theme-support-20121016.diff (2.7 KB) - added by alexkingorg 3 years ago.
add-remove-theme-support-20121017.diff (3.1 KB) - added by alexkingorg 3 years ago.

Download all attachments as: .zip

Change History (28)

#1 follow-up: @scribu
3 years ago

  • Keywords 2nd-opinion added

-1 on a boolean third parameter.

But more importantly, why are you calling add_theme_support() from a plugin? It makes zero sense to me.

#2 in reply to: ↑ 1 @toscho
3 years ago

  • Cc info@… added

Replying to scribu:

But more importantly, why are you calling add_theme_support() from a plugin? It makes zero sense to me.

Because if you add thumbnail to the supports argument in register_post_type() the theme support will not be activated automatically, and you get not thumbnail meta box in the edit screen. But you may need exactly that in custom templates.

#3 @alexkingorg
3 years ago

WIthout the boolean 3rd parameter, how do you propose determining if the passed value should merge or stomp?

In the next version of Twitter Tools:


tweets are downloaded and stored as a custom post type. I am enabling Featured Images for just that post type.

#4 @markoheijnen
3 years ago

Third parameter doesn't make sense to me too. You then should be able to do remove_theme_support( 'post-thumbnails', array( 'my-new-post-type' ) );

I can see the usage. I ones needed thumbnail support to a post type because it is used for a mobile app. That isn't really a theme thing.

#5 @alexkingorg
3 years ago

add_theme_support is an API, the usage example in the codex is:

add_theme_support( 'post-thumbnails' );
add_theme_support( 'post-thumbnails', array( 'post' ) );          // Posts only
add_theme_support( 'post-thumbnails', array( 'page' ) );          // Pages only
add_theme_support( 'post-thumbnails', array( 'post', 'movie' ) ); // Posts and Movies

I mistakenly understood this to mean I could add support for other post types by passing them in. I believe others will make the same mistake. The code in the Gist that "fixes" this is simple but was decidedly non-trivial to discover. Requiring that type of interaction with an API makes it likely for people to make mistakes. I think making it easier for them is better.

The action this API currently takes for post-thumbnails would be better named as set_theme_support() rather than add_theme_support().

I also agree that remove_theme_support should accept a post type param for post-thumbnails, but didn't have time to add that to the patch yet.

#6 @scribu
3 years ago

Actually, why do we need to specify if a custom post type supports thumbnails in two places?

To enable thumbnails for pages, this should be enough:

add_post_type_support( 'page', 'thumbnails' );
add_theme_support( 'post-thumbnails' );

And for CPTs you define yourself, you could just set the 'supports' array directly.

Last edited 3 years ago by scribu (previous) (diff)

#7 @lightningspirit
3 years ago

  • Cc lightningspirit@… added

If a theme doesn't support thumbnails by default, no plugin should deauthorize that rule. UI is responsability of the theme and plugins should only extend functionality.
In the case a plugin adds functionality that needs thumbnails and a theme doesn't provide thumbnails, should simply throw an message or notice.
This is my opinion.

#8 @nacin
3 years ago

I doubt that a "no stomp" argument is necessary.

The function is "add", not "set". If you want to remove, you should call remove. We've been careful to merge arguments for other theme support arguments (namely, custom headers and backgrounds), so this patch would make things consistent.

It would make sense that when doing this, remove_theme_support() is updated to handle removing individual post types.

scribu: You need to support it in two places because just because the post type supports thumbnails for a type, doesn't mean the theme does.

#9 follow-up: @alexkingorg
3 years ago

Ok, I'm reading this as: please revise the patch as follows:

  • no "stomp/replace" param (actually makes things simpler)
  • add matching functionality to be able to remove post-thumbnails on a per post-type basis

I generally agree and will proceed with those changes.

The only edge case I can think of is someone trying to explicitly remove support for post-thumbnails from a specific post type when a blanket "support it for all post types" has been set. I'm leaning towards that "doing nothing" and returning an error/false-ish response. Anyone else have a good idea for this situation?

#10 in reply to: ↑ 9 @scribu
3 years ago

Replying to alexkingorg:

The only edge case I can think of is someone trying to explicitly remove support for post-thumbnails from a specific post type when a blanket "support it for all post types" has been set.

They can just call remove_post_type_support( 'foo', 'thumbnails' ), no?

Last edited 3 years ago by scribu (previous) (diff)

#11 @alexkingorg
3 years ago

Good idea, I think that's a solid workaround.

#12 @alexkingorg
3 years ago

Ok, here is the patch with matching support for remove_theme_support( 'post-thumbnails' );

I think a couple of unit tests for this would make sense, so I'll be working on those as well.

#13 @alexkingorg
3 years ago

Unit tests were a good idea, there are bugs in that last patch. Will update shortly. Apologies for the noise here.

#14 @alexkingorg
3 years ago

Ok, the new patch is now working for me with the unit tests attached here:


#15 @nacin
3 years ago

  • Keywords 3.6-early added; 2nd-opinion removed
  • Milestone changed from Awaiting Review to Future Release
  • Type changed from defect (bug) to enhancement
  • Version changed from trunk to 2.9

#16 @husobj
3 years ago

  • Cc ben@… added

#17 @flyingtrolleycars
3 years ago

  • Cc andrew@… added

#18 @alexkingorg
3 years ago

I don't suppose this is going to make it into 3.6 at this point?

#19 follow-up: @nacin
3 years ago

Related, #21912. Yeah, I kind of hate the current behavior. Find me on the first day of the 3.7 cycle and I will commit it. I think beta is too late for a change in behavior. It would be interesting to brainstorm common use cases where this might break, so we can pre-empt those with blog posts.

#20 @nacin
3 years ago

Maybe the filter proposed in #21912 is a good idea after all.

#21 in reply to: ↑ 19 @husobj
3 years ago

Replying to nacin:

Find me on the first day of the 3.7 cycle and I will commit it.

Is it the first day of 3.7 now?

#22 @husobj
3 years ago

Just to add in another couple of reasons why I think this fix is needed:

  1. If you build a child theme, you may add additional post-thumbnails support for post types that are not supported by the parent theme, therefore you would expect add_theme_support() to merge rather than replace.
  1. Many plugins (ecommerce is a particularly good example) now add their own 'theming' structure so they can work out-of-the-box with no template changes. It would be ideal if they could benefit from using the core Post Thumbnail admin UI without having to resort to a custom solution.

#23 @SergeyBiryukov
2 years ago

#27254 was marked as a duplicate.

#24 @chriscct7
4 months ago

  • Keywords needs-refresh added; 3.6-early removed

#25 @jmichaelward
4 months ago

Hello everyone,

I am proposing revisiting this ticket, because I believe the current implementation of add_theme_support( 'post-thumbnails' ) to be somewhat confusing at best, and at worst, it may introduce conflicts between plugins and themes.

As an example, I was reviewing the current version of WooCommerce (version 2.4.7 at the time of this writing) when I noticed that line 348 of woocommerce.php declares a function titled add_thumbnail_support(). This function runs a check to see whether the currently-installed theme supports post thumbnails. If it doesn't, the plugin adds post thumbnail theme support for all installed post types, then attaches that theme support the WooCommerce product post type, as well.

This could be potentially confusing to an end user who did not have post thumbnail support previously activated in their theme, and upon activated WooCommerce, now sees the option to add featured images to all of their pages, posts, and any other custom post type that is registered in the system. Furthermore, should they choose to add a featured image to those previously-unsupported post types, confusion may be compounded when they do not see those images displayed on the front-end.

Currently, add_theme_support( 'post-thumbnail' ) accepts an additional array value for which post types should include post thumbnail support. Ideally, plugin authors would be able to do something along these lines:

add_theme_support( 'post-thumbnails', array( 'my_custom_post_type' );

...which would append their custom post type to an array of other post types that have post thumbnail support enabled. If none are enabled, this would register post thumbnail support and assign it to that post type. If, at that time, the end user installs another plugin or a theme that adds specified post thumbnail support in this manner, there would be no conflicts as to which post types would be displaying the option to attach a featured image.

I just came across this issue today, so I don't have a proposed resolution just yet, but I do think we can improve upon the previously solutions that have been proposed. I'll plan to write something up and submit it in the next few days. In the meantime, I wanted to re-open this issue for discussion.

Note: See TracTickets for help on using tickets.