Make WordPress Core

Opened 11 years ago

Closed 11 years ago

Last modified 10 years ago

#26321 closed defect (bug) (invalid)

Warning message for update_post_thumbnail_cache()

Reported by: klihelp's profile klihelp Owned by: wonderboymusic's profile wonderboymusic
Milestone: Priority: normal
Severity: normal Version: 3.8
Component: Cache API Keywords: 3.9-early needs-unit-tests reporter-feedback has-patch
Focuses: Cc:

Description

I got a warning/error related to update_post_thumbnail_cache() function:

Warning: Invalid argument supplied for foreach() in ./wp-includes/post-thumbnail-template.php on line 64

The $wp_query->posts array is missing, but we have the single $wp_query->post.

foreach ( $wp_query->posts as $post ) {
	if ( $id = get_post_thumbnail_id( $post->ID ) )
		$thumb_ids[] = $id;
}

Attachments (1)

26321.patch (465 bytes) - added by SergeyBiryukov 11 years ago.

Download all attachments as: .zip

Change History (20)

#1 @SergeyBiryukov
11 years ago

  • Keywords reporter-feedback added

Could you provide a piece of code or the steps to reproduce the issue?

update_post_thumbnail_cache() is only called in the loop, i.e. after the_post(). Why would $wp_query->posts be empty?

#2 @klihelp
11 years ago

It's happening with custom WP_Query() loop on pages where is_page() == TRUE.

Using Woocommerce plugin shortcodes on pages, warning message only appears once before the first product thumbnail, but thumbnail displayed correctly.

Maybe related to #19949

Last edited 11 years ago by klihelp (previous) (diff)

#3 @klihelp
11 years ago

  • Keywords reporter-feedback removed

#4 @klihelp
11 years ago

This simple patch should be in WP 3.8

#5 @markoheijnen
11 years ago

  • Keywords 3.9-early needs-unit-tests reporter-feedback added

This first needs some unit tests because maybe there is a valid reason why this is happening. Also I'm curious if you can reproduce this on a clean installation without plugins like WooCommerce since they can be part of the issue.

#6 @klihelp
11 years ago

This is happening on clean installation too, eg. page.php displays the_post_thumbnail() inside custom WP_Query() loop.

#7 @wonderboymusic
11 years ago

  • Milestone changed from Awaiting Review to 3.9

#8 @wonderboymusic
11 years ago

  • Keywords has-patch added

#9 @wonderboymusic
11 years ago

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

In 27166:

Don't iterate over $wp_query->posts in update_post_thumbnail_cache() if it is empty. Adds unit tests.

Props SergeyBiryukov, for the original patch.
Fixes #26321.

#10 @nacin
11 years ago

This would not set thumbnails_cached to true. It's possible someone is checking that de facto public property, no?

I was not aware of a situation where $wp_query->posts is empty but $wp_query->post is not. Though perhaps if that is actually something that occurs, we should return false after all? (Ideally, posts[0] actually gets populated with post, but that's a separate thing.)

#11 @wonderboymusic
11 years ago

@nacin was weirded out that this could happen - @klihelp, could you explain a little more about how this code ran outside of the loop?

#12 @bpetty
11 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

This unit test assertion consistently fails:

$this->assertFalse( $GLOBALS['wp_query']->thumbnails_cached );

See unit test results:

1) Tests_Post_Thumbnails::test_update_post_thumbnail_cache
Failed asserting that true is false.

/home/bryan/Projects/wp-vagrant/wordpress/tests/phpunit/tests/post/thumbnails.php:13

#13 @wonderboymusic
11 years ago

In 27183:

Reset $wp_query and $post during WP_UnitTestCase::tearDown() to avoid spillage of globals between tests.

See #26321.

#14 @klihelp
11 years ago

@klihelp, could you explain a little more about how this code ran outside of the loop?

@wonderboymusic, I run into this one with WooCommerce plugin, showing a list of Products inside a Page (eg. custom loop inside page.php)

#15 @nacin
11 years ago

Given WooCommerce is pegged as the culprit here, I'd like to revert [27166] (and [27183]). This is appears to be a legitimate warning caused by $posts being improperly set. If you're going to mess with WP_Query's internals, don't set $post without setting $posts.

Last edited 11 years ago by nacin (previous) (diff)

#16 @nacin
11 years ago

[27183] can stay.

#17 @nacin
11 years ago

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

In 27746:

Revert [27166].

We should not be accounting for improper assignment of WP_Query properties.

fixes #26321.

#18 @nacin
11 years ago

  • Milestone 3.9 deleted
  • Resolution changed from fixed to invalid

Some might ask why this was reverted. Basically, it's a maintenance burden we don't want. If we're going to account for $post being set and $posts missing in this one instance, we'll need to consciously deal with it elsewhere, too. That quickly becomes a slippery slope and it's too much of a burden to deal with code that does it wrong.

#19 @kanenas
10 years ago

I am using WordPress v.3.9.1 (with NO modifications to WordPress Core) with "BUCKET - A Digital Magazine Style WordPress Theme" (http://themeforest.net/item/bucket-a-digital-magazine-style-wordpress-theme/6107209) and I runned into this...

PHP Warning:  Invalid argument supplied for foreach() in /public_html/wp-includes/post-thumbnail-template.php on line 64

See the whole discussion here http://wordpress.stackexchange.com/questions/152022/get-the-post-thumbnail-invalid-argument-supplied-for-foreach-in-wp-includ/

The patch (26321.patch) did correct the problem!

Note: See TracTickets for help on using tickets.