WordPress.org

Make WordPress Core

#37697 closed defect (bug) (fixed)

Strange behavior with thumbnails on preview in 4.6

Reported by: jbenton Owned by: joemcgill
Milestone: 4.6.1 Priority: high
Severity: normal Version: 4.6
Component: Post Thumbnails Keywords: has-patch has-unit-tests commit fixed-major
Focuses: Cc:

Description (last modified by swissspidy)

Since upgrading my site to 4.6, we've had strange behavior regarding featured images on preview. Whenever a preview_nonce appears in a preview URL, an additional &_thumbnail_id=xxxxxxx appears. And at that point, all featured-image thumbnails on the page become that specific image — even when they appear in a different loop or are called through other means. @swissspidy says on Twitter this may be related to this change in 4.6: [38118]

Adding a wp_reset_postdata() does not seem to fix the problem. Screenshots and code available in this tweets in this Twitter thread:

https://twitter.com/swissspidy/status/766004453807882244

Attachments (4)

37697.diff (1.2 KB) - added by joemcgill 22 months ago.
37697.2.diff (2.2 KB) - added by swissspidy 22 months ago.
37697.3.diff (2.0 KB) - added by swissspidy 22 months ago.
Remove file upload
37697.4.diff (2.1 KB) - added by joemcgill 22 months ago.

Download all attachments as: .zip

Change History (21)

#1 @swissspidy
22 months ago

  • Description modified (diff)

#2 @joemcgill
22 months ago

  • Keywords needs-patch needs-unit-tests added
  • Owner set to joemcgill
  • Priority changed from normal to high
  • Status changed from new to accepted

@joemcgill
22 months ago

#3 @joemcgill
22 months ago

  • Keywords needs-testing added; needs-patch needs-unit-tests removed
  • Milestone changed from Awaiting Review to 4.6.1

@jbenton Thanks for the ticket. I noticed this bug at almost the same time you did, I think. The issue is that the new _wp_preview_post_thumbnail_filter() function fails to check if the $post_id is from the post we're previewing, so it can affect any secondary loops on the preview page.

37697.diff adds an additional check to make sure we're only affecting the main post object from the preview page and updates the unit test for this function so it includes the check as well.

#4 @joemcgill
22 months ago

For those wanting to test this issue, you'll need to add something like the below code after the main loop on a custom page template and then preview a page using that template.

<?php
$test_args = array(
        'post_type'  => 'post',
        'posts_per_page' => 4,
        'meta_query' => array(
                array(
                        'key' => '_thumbnail_id',
                        'compare' => 'EXISTS'
                ),
        ),
        'ignore_sticky_posts' => true,
);

$stories = new WP_Query( $test_args );

while ( $stories->have_posts() ) {
        $stories->the_post();
        if ( has_post_thumbnail() ) {
                the_post_thumbnail();
        }
}

wp_reset_postdata();
?>

This ticket was mentioned in Slack in #core-images by joemcgill. View the logs.


22 months ago

@swissspidy
22 months ago

#6 @swissspidy
22 months ago

  • Keywords has-patch has-unit-tests added

There needs to be a unit test showing the effect of the additional $post_id != $_REQUEST['preview_id'] check.

In 37697.2.diff I added second unit test to demonstrate the fixed behaviour. I also added an empty( $_REQUEST['preview_id'] ) check to _wp_preview_post_thumbnail_filter() and simplified the tests a bit.

  • resets inside the tests should happen before the assertions. If an assertion fails, the following could won't be called
  • simply doing $GLOBALS['post'] = $old_post; should be enough there

@swissspidy
22 months ago

Remove file upload

@joemcgill
22 months ago

#7 @joemcgill
22 months ago

37697.4.diff is similar to 37697.3.diff, but adds some additional cleanup to the unit tests to avoid potential test spillage.

#8 @swissspidy
22 months ago

#37711 was marked as a duplicate.

#9 @swissspidy
22 months ago

@jbenton do you perhaps have time to test this patch on your site to see if it properly fixes the bug?

#10 @joemcgill
22 months ago

#37729 was marked as a duplicate.

This ticket was mentioned in Slack in #core-images by joemcgill. View the logs.


21 months ago

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


21 months ago

#13 @swissspidy
21 months ago

  • Keywords commit added

We should get this into trunk to get more feedback.

#14 @joemcgill
21 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 38433:

Post Thumbnails: Prevent post thumbnail previews from spilling into other images.

After [38118], when previewing a page with a secondary loop, all post
thumbnails would be filtered to display the post thumbnail for the
page being previewed. This ensures _wp_preview_post_thumbnail_filter()
is only applied if the $post_id of the post meta being filtered is
equal to the post or page being previewed.

Props swisspidy, joemcgill.
Fixes #37697.

#15 @joemcgill
21 months ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Fixed in trunk. Reopening for minor.

#16 @jeremyfelt
21 months ago

  • Keywords needs-testing removed

I've reproduced this locally and [38433] resolves the issue. It might be nice to break the conditional up a bit in the future so that the properties of $post are checked separately from the existence and values of _thumbnail_id and preview_id, but that's only a readability preference, not a functional concern.

Standard and multisite tests are passing.

#17 @jeremyfelt
21 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 38476:

Post Thumbnails: Prevent post thumbnail previews from spilling into other images.

After [38118], when previewing a page with a secondary loop, all post
thumbnails would be filtered to display the post thumbnail for the
page being previewed. This ensures _wp_preview_post_thumbnail_filter()
is only applied if the $post_id of the post meta being filtered is
equal to the post or page being previewed.

Merge of [38433] to the 4.6 branch.

Props swissspidy, joemcgill.
Fixes #37697.

Note: See TracTickets for help on using tickets.