Make WordPress Core

Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#37658 closed defect (bug) (fixed)

Featured image on media files can not be changed

Reported by: clorith's profile Clorith Owned by: ocean90's profile 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)

37658.diff (1.3 KB) - added by flixos90 8 years ago.
37658.2.diff (3.8 KB) - added by flixos90 8 years ago.
37658.3.diff (3.9 KB) - added by flixos90 8 years ago.
37658.4.diff (4.4 KB) - added by flixos90 8 years ago.
37658.5.diff (4.5 KB) - added by flixos90 8 years ago.

Download all attachments as: .zip

Change History (20)

@flixos90
8 years ago

#1 @flixos90
8 years ago

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 and attachment:video).

@flixos90
8 years ago

#2 @flixos90
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.

@flixos90
8 years ago

#4 in reply to: ↑ 3 ; follow-up: @flixos90
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 @ocean90
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 @ocean90
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

@flixos90
8 years ago

#9 @flixos90
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.

#10 @ocean90
8 years ago

  • Milestone changed from Awaiting Review to 4.6

#11 @ocean90
8 years ago

  • Focuses javascript removed
  • Keywords commit added

37658.4.diff looks good to me.

@flixos90
8 years ago

#12 @flixos90
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.

This ticket was mentioned in Slack in #core by ocean90. View the logs.


8 years ago

#14 @ocean90
8 years ago

  • Owner set to ocean90
  • Resolution set to fixed
  • Status changed from new to closed

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.

#15 @ocean90
8 years ago

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.

Note: See TracTickets for help on using tickets.