#37658 closed defect (bug) (fixed)
Featured image on media files can not be changed
Reported by: | Clorith | Owned by: | ocean90 |
---|---|---|---|
Milestone: | 4.6 | Priority: | normal |
Severity: | normal | Version: | 4.6 |
Component: | Post Thumbnails | Keywords: | has-patch has-unit-tests commit |
Focuses: | Cc: |
Description
When going to Media and selecting to edit the details of a non-image media file (such as an .mp3), you can select a featured image for the file.
This no longer saves the new image you've changed it to, but works as expected in 4.5.
I suspect it's related to r38118, as it appears the featured image box here is piggybacking off wp.media.post
Attachments (5)
Change History (20)
#2
@
8 years ago
- Keywords has-patch has-unit-tests added
In 37658.2.diff I added a unit test for the issue. I also added two more general unit tests for 38118 which was missing unit tests.
#3
follow-up:
↓ 4
@
8 years ago
I looked through the code base and found three similar checks:
- https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/image.php?rev=37488&marks=136,139#L134
- https://core.trac.wordpress.org/browser/trunk/src/wp-admin/edit-form-advanced.php?rev=38029&marks=58,61,63#L57
- https://core.trac.wordpress.org/browser/trunk/src/wp-includes/media.php?rev=38086&marks=3328,3331,3333#L3327
All of them are using an additional current_theme_supports()
check. This is missing in [38118] and 37658.2.diff. Based on [27657] we should check for both right?
/cc: @wonderboymusic
#4
in reply to:
↑ 3
;
follow-up:
↓ 5
@
8 years ago
Replying to ocean90:
I looked through the code base and found three similar checks:
Ah, I missed that somehow. It's fixed in 37658.3.diff. Out of curiosity: Why does the initial post type vs theme support check use &&
while the attachment subtype ones use ||
?
#5
in reply to:
↑ 4
@
8 years ago
Replying to flixos90:
Out of curiosity: Why does the initial post type vs theme support check use
&&
while the attachment subtype ones use||
?
It looks like it was OR in [27657] but [28051] changed it back to AND. See also ticket:27673:6.
#6
@
8 years ago
- Keywords needs-refresh added
37658.3.diff: Two of three tests are currently failing:
1) Tests_Post_Thumbnail_Template::test__wp_preview_post_thumbnail_filter Failed asserting that '' matches expected 4. /tests/phpunit/tests/post/thumbnails.php:247 2) Tests_Post_Thumbnail_Template::test_insert_attachment_with_post_thumbnail Failed asserting that '' matches expected 4. /tests/phpunit/tests/post/thumbnails.php:293
This ticket was mentioned in Slack in #core by ocean90. View the logs.
8 years ago
This ticket was mentioned in Slack in #core-images by joemcgill. View the logs.
8 years ago
#9
@
8 years ago
- Keywords needs-refresh removed
37658.4.diff fixes the failing tests.
As discussed on Slack, the first failure was caused by the global $post
not being properly set up.
The second failure was happening because setting the post thumbnail was done before setting the attached file for an attachment. When inserting a new attachment, that was causing wp_attachment_is()
to return false because no file was set at that point. It wouldn't have affected existing attachments, but now it's fixed for inserting new ones as well, by moving the post thumbnail block below setting the attached file.
#12
@
8 years ago
After @joemcgill outlined that using an image file for an audio attachment does not make a lot of sense, I changed that in 37658.5.diff. It's a minor adjustment that wouldn't cause problems before, but it might at some point when changing related internals of WordPress - so this way it should be more error-proof.
37658.diff fixes this issue. The problem was that the check for post type support ignored the more specific subtype checks we need for attachments (
attachment:audio
andattachment:video
).