Setting featured image does not require the post to be saved

[rooodini]

This may not be a bug... But it is not intuitive behaviour (I'd expect it to work more like Post Tags).

It's probably quite clear, but here's a test case anyway:

1. Create a new draft post
2. Enter a title
3. Click "Save draft"
4. Click "Set featured image", pick an image, click "Use as featured image"
5. Close the dialog
6. Navigate away and come back (without clicking "Save draft" or "Publish"). The featured image has been added to the post.

[dd32]

As with all operations in the media dialogue, the operation happens when you click on the action links.

Aside from inserting data into the post content (images/gallery) all operations are saved at that point in time.

Thats the expected behaviour to me.

[nacin]

I'd agree. It's expected behavior, or at least has been for a while now, and it certainly won't change for 3.0. I say that because media/upload will probably be a big 3.1 project, and most of the UI will change.

[rooodini]

Yeah I had in mind that this would just be for a future release.

I take your point about the media dialogue.

I prefer the way post tags work.. I click "Add tag", the tag appears to have been changed, but nothing is saved until I click Save or Publish.

The featured image seems more related to the post and less to general media. That's why I imagine it working more like post tags and less like the rest of the media dialogue.

[hakre]

Both sides are somehow right. Wontfix?

[scribu]

#17100 was closed as duplicate.

It highlights the fact that all custom fields behave this way.

[nacin]

All custom fields do behave this way, but we can make it so featured images don't. The patch on #21776 handles this, so closing this as a duplicate.

[mboynes]

This appears to still be an issue. I looked around for more recent information on this issue, but failed to find any. Not sure if there was a regression since #21776 or if that didn't resolve it as advertised.

[SergeyBiryukov]

#33505 was marked as a duplicate.

[flixos90]

While I see the point that setting the thumbnail without a page reload is not in line with the rest of the Edit Post data, I don't think we should change that, especially with a possible revamp of the post editor in mind (AJAX saving, see #7756) where the current implementation should already be good for.

Also, any user that relies on the featured image being managed directly via AJAX, would be confused if this suddenly stopped working.

[flixos90]

We had some further discussions (at the Contributor Day in Nuremberg) and came to the conclusion that we can probably fix this without affecting too many users (contrary to my previous comment). Most people aren't aware that this bug even exists, and they assume that managing the featured image works in the way that this ticket is hinting it should.

About the previous concerns whether this is expected behavior or not: While generally media-related changes are saved immediately with AJAX, this shouldn't be the case in the context of posts. The featured image does not change the attachment itself, but the post that it is attached to (similar like inserting media into the post content). Therefore it should only do something on hitting the "Update" button.

To fix the ticket, we should move away from managing the featured image through AJAX and instead use a hidden input field to store the image ID (or 0 for no featured image or to remove the existing featured image). The POST data this would send could then be handled in edit_post() (in a similar fashion to the current AJAX implementation).

[flixos90]

patch to only save featured image on "Update"

[flixos90]

12922.diff​ is a take on this ticket.

The featured thumbnail ID is stored in a hidden input field with name _thumbnail_id which is submitted when hitting the "Update" button and then handled by edit_post().

The AJAX function for set-post-thumbnail is no longer used, instead a new AJAX function for get-post-thumbnail-html is used to render the metabox content (basically providing a preview, similar to the original set-post-thumbnail function, only without actually modifying the post).

Additionally I moved all thumbnail management for the post editor into the media-editor.js file so that we don't need to use the global JS function WPRemoveThumbnail any longer.

[flixos90]

This bug was supposed to be fixed in 3.5. I will compare 3.5 with trunk to test their behaviors.

[flixos90]

Reference: This bug was fixed in 21770 and then reappeared after 22979.

[flixos90]

After comparing, the initial patch appears to integrate the original changes from 21770 into trunk well. In 12922.3.diff​ I did some fine tuning by removing duplicate code and fixing some docs.

[flixos90]

12922.4.diff​ has some minor improvements in the wp_ajax_get_post_thumbnail_html() function.

[swissspidy]

Additionally I moved all thumbnail management for the post editor into the media-editor.js file so that we don't need to use the global JS function WPRemoveThumbnail any longer.

Should we remove/deprecate this function then? It's not used anywhere else.

[flixos90]

Replying to swissspidy:

Additionally I moved all thumbnail management for the post editor into the media-editor.js file so that we don't need to use the global JS function WPRemoveThumbnail any longer.

Should we remove/deprecate this function then? It's not used anywhere else.

Sounds good to me. Also, we might consider doing something similar to wp_ajax_set_post_thumbnail().

[ocean90]

It seems like the latest patch doesn't handle the "Preview Changes" button for published posts. Possibly related to #20299.

[westonruter]

BTW: The ​Customize Posts plugin allows for a featured image change to be fully previewed in the Customizer. From first glance at the patch on this ticket, it also changes the behavior of the Featured Image metabox on the edit post screen to prevent the featured image from being set as soon as it is selected: instead, it populated a hidden input with the attachment ID which then gets handled during a save_post action. See ​https://github.com/xwp/wp-customize-posts/blob/0.6.1/php/class-wp-customize-featured-image-controller.php

[flixos90]

Replying to westonruter:

BTW: The ​Customize Posts plugin allows for a featured image change to be fully previewed in the Customizer. From first glance at the patch on this ticket, it also changes the behavior of the Featured Image metabox on the edit post screen to prevent the featured image from being set as soon as it is selected: instead, it populated a hidden input with the attachment ID which then gets handled during a save_post action.

Not setting the featured image immediately was the original cause for this ticket. When we get this fixed, the Customize Posts plugin needs to be adjusted to that as well I think. If you have any concerns that this would not be possible with the approach of the current patch, please let me know.

Replying to ocean90:

It seems like the latest patch doesn't handle the "Preview Changes" button for published posts.

I will look at this by the end of this week. If it is indeed caused by to #20299, can we ignore it for the sake of this ticket? Or do we need to workaround?

[westonruter]

Replying to flixos90:

Not setting the featured image immediately was the original cause for this ticket. When we get this fixed, the Customize Posts plugin needs to be adjusted to that as well I think. If you have any concerns that this would not be possible with the approach of the current patch, please let me know.

Yes, Customize Posts can disable its implementation once it is in core. I shared the code from the Customize Posts implementation in case it is helpful for you.

[ocean90]

Replying to flixos90:

Replying to ocean90:

It seems like the latest patch doesn't handle the "Preview Changes" button for published posts.

I will look at this by the end of this week. If it is indeed caused by to #20299, can we ignore it for the sake of this ticket? Or do we need to workaround?

Not updating the featured image in a preview doesn't sound like a good idea.

[flixos90]

I have two updated patches which take different approaches on fixing the previewing issues:

12922.5.diff​ actually uses revision post meta to store the post thumbnail. It does not open up post meta to revisions generally, but adds an exception for the _thumbnail_id meta key. The patch ensures that this meta field is stored as revision meta for all revisions and also covers autosaving.

12922.6.diff​ uses a similar approach like we currently have for the post format. It simply passes the current value of the _thumbnail_id to the preview as a query argument.

After having thought about and tested both implementations, I think we should rather go with the latter for now. While it is more a workaround than an actual solution, we can still change it when we actually start supporting revision meta at some point (see #20299). The first approach works as well, but I'm not really fond of using revision meta for this change before we generally open it up, especially since this would include adding all these exceptions for the _thumbnail_id meta key.

Furthermore the revision meta approach could easily cause other issues we wouldn't wanna deal with that late in Beta cycle. The second one is more straightforward, so we might be able to deliver it with 4.6.

[Kelderic]

I noticed this when I was creating the Featured Galleries plugin, which behaves very similar to Featured Images. I solved it by storing two separate DB entries. A temp and a perm. The temp is updating via AJAX, and it is substituted into pages if the page is currently a preview. The perm value is updating when the page is saved. Finally whenever the page is opened, the temp is overwritten by the perm.

I'm not sure if this would have a huge impact on DB size, but it does solve all problems with previewing.

[joemcgill]

Replying to flixos90:

After having thought about and tested both implementations, I think we should rather go with the latter for now. While it is more a workaround than an actual solution, we can still change it when we actually start supporting revision meta at some point (see #20299). The first approach works as well, but I'm not really fond of using revision meta for this change before we generally open it up, especially since this would include adding all these exceptions for the _thumbnail_id meta key.

I had the same reservations about adding exceptions for _thumbnail_id to revision post meta when I was looking over the two approaches. The approach to use query variables for previews in 12922.6.diff​ definitely feels like a workaround to me, but is a decent improvement until there is a better approach for handling revision post meta, because it gives people the ability to preview featured image changes on published posts without accidentally updating the post.

Previewing changes to drafts still seems to save any changes to the featured image, but I think it's fine to leave that behavior since the goal here is to keep someone from accidentally changing the image of their published post without realizing it.

[ocean90]

Let's do 12922.6.diff​. I've only a couple of minor things which can be fixed on commit:

• get-post-thumbnail-html should be added to the end of the list in admin-ajax.php.
• "backwards compatibility" without "s", see ​https://make.wordpress.org/core/handbook/best-practices/spelling/.
• Can we add a short comment above the code in wp-includes/post.php? Something like // Add or remove featured image.
• _wp_preview_post_thumbnail_filter(): In the DocBlock, between the name and the description of a variable one space should be removed, there should be only one (and the others aligned).
• Yoda conditions for <, >, <= or >= should be avoided, see ​https://make.wordpress.org/core/handbook/best-practices/coding-standards/php/#yoda-conditions.
• It seems like $single is unused in _wp_preview_post_thumbnail_filter(). It should be removed. (The add_filter() call needs to be adjusted too, 4 => 3)

[flixos90]

12922.7.diff​ is an updated patch that takes care of the above issues.

[joemcgill]

In 38118:

Post Thumbnails: Only update featured images when saving a post.

Previously, changing the post thumbnail of a published post in the edit screen
would immediately apply the change, rather than waiting for the post to be
saved before applying the update. This could lead to someone unintentionally
editing the post thumbnail on a published post, and made it impossible to
preview changes to post thumbnails on published posts before saving the change.

This introduces a new Ajax handler, wp_ajax_get_post_thumbnail_html() to
retrieve the HTML for the post thumbnail meta box without updating the post
meta value for _thumbnail_id. It also allows post thumbnail changes to be
previewed by passing the _thumbnail_id as a query variable to the preview
screen and adding a new filter, _wp_preview_post_thumbnail_filter(), which
gets applied to get_post_metadata during the post preview process.

Props flixos90.
Fixes #12922.

[ocean90]

In 38137:

Post Thumbnails: Remove an unused nonce in _wp_post_thumbnail_html().

See #12922.

[ocean90]

In 38263:

Post Thumbnails: Restore thumbnail support for media files.

• Allow to add/remove a featured image to attachment:audio and attachment:video post types, see [27657].
• Change conditionals to check for theme OR post type support.
• Add tests for #12922.

Broken in [37658].

Props flixos90, joemcgill, DrewAPicture, wonderboymusic.
See #12922.
Fixes #37658.

[ocean90]

In 38264:

Post Thumbnails: Restore thumbnail support for media files.

• Allow to add/remove a featured image to attachment:audio and attachment:video post types, see [27657].
• Change conditionals to check for theme OR post type support.
• Add tests for #12922.

Broken in [37658].

Merge of [38263] to the 4.6 branch.

Props flixos90, joemcgill, DrewAPicture, wonderboymusic.
See #12922.
See #37658.