Why X Theme and Pro sometimes cause PHP warnings on WPEngine

The Problem

Earlier this fall I was working on an e-commerce site that’s hosted on WPEngine that used Themeco’s Pro as its parent theme most pages hadn’t seen any customization yet, most of the code was stock, and this appeared on the Products page:

PHP Warning:  Invalid argument supplied for foreach() in wp-content/mu-plugins/wpengine-common/plugin.php

Hmm. Who is passing what to a foreach?

It turns out that both X Theme and it’s progeny Pro do this turn off WordPress’s automatic responsive images:

// Disable WordPress 4.4 Responsive Images
// =============================================================================

if ( ! function_exists( 'x_disable_wp_image_srcset' ) ) :
  function x_disable_wp_image_srcset( $source ) {
    return false;
  }
  add_filter( 'wp_calculate_image_srcset', 'x_disable_wp_image_srcset' );
endif;

They’re hooking into the wp_calculate_image_srcset filter and returning false, which stops WordPress from adding a srcset attribute to an image, (more on why in a moment). The problem here is that the value being passed to the filter, $sources, is an array, but x_disable_wp_image_srcset() turns that array into a boolean false, so then anything else hooking into wp_calculate_image_srcset, for example, WPEngine’s CDN system, tries to iterate over the $sources array, but it’s not an array, hence the error. By filtering $sources to a boolean false Themeco is breaking the expected, and documented, behaviour of the wp_calculate_image_srcset filter.

Solutions

Luckily, there are two easy fixes for this problem. The first is to make sure you’re using the latest version of WPEngine’s must-use plugin. They’ve caught on to the problem and check to make sure $sources is actually an array:

if ( is_array( $sources ) ) {
    foreach ( $sources as $source ) {
        // do the stuff
    }
}

If you can’t do that, at least Themeco made the function pluggable, so you can add your own version of x_disable_wp_image_srcset() to your theme’s functions.php or a plugin that returns an empty array and plays nicely with other plugins:

// This is a pluggable function. If we don't define it here then Pro does in 
// pro/framework/functions/global/admin/thumbnails/setup.php
function x_disable_wp_image_srcset( $sources, $size_array, $image_src, $image_meta, $attachment_id ) {
    return [];
}

Or, you could use responsive images.

Confusion, (aka, why this happens in the first place)

When I first discovered this problem I E-mailed Themeco to tell them about the problem, however they pointed me at the documentation for the wp_calculate_image_srcset() function, which states the function can return false, and sent me on my way:

Hey John,

Thanks for writing in. In looking over WordPress' official documentation for that function/hook, I believe that boolean false should be the correct value to return:

https://developer.wordpress.org/reference/functions/wp_calculate_image_srcset/

In the "Return" section you'll see that it is supposed to return a string or a boolean false if there is an "error or only one source exists." It seems as though WPEngine should need to revise their function to account for these instances anyway even if not for X or Pro (and at least work to catch any fatal errors like this).

Hopefully that helps, cheers.


Best Regards,
Your Themeco Team

This confusion occurs because the convention in WordPress is that if a filter and a function have the same name, the filter filters the output of the function. However, in this case the convention is broken – when I raised the issue in the Advanced WordPress Facebook group a very experienced WP developer made the exact same mistake and assumed that allowable return types for the function should be fine to return from the filter. After we cleared up the confusion I filed a bug about the confusing names, go star it so it gets some attention!

A Fluke of PHP

So why does returning false in the wp_calculate_image_srcset filter stop WordPress from using responsive images? Here’s the relevant code from media.php:

$sources = apply_filters( 'wp_calculate_image_srcset', $sources, $size_array, $image_src, $image_meta, $attachment_id );

// Only return a 'srcset' value if there is more than one source.
if ( ! $src_matched || count( $sources ) < 2 ) {
    return false;
}

To follow along, if $sources is turned into a boolean false at the apply_filters() call, then a count( false ) happens in the condition on the next non-comment line. Instead of an error like I expected, running count() on any non-array value returns 1, so count( false ) == 1, also count( 0 ) == 1

Conclusion

First, be a good citizen of the WordPress ecosystem and don’t change the type of a filtered value to something other than the documented types for that value. Second, if you have any non-core themes or plugins installed you can’t trust that a value coming into a filter will be the type that the docs say, so check! When WPEngine realized there was a problem an is_array() fixed the problem. Third, read support E-mails thoroughly and make sure you understand them, it hurts when I go to the effort to document a problem and come up with a solution, and I’m told it’s not a problem. Finally, go +1 my ticket so maybe others won’t get bitten.

What do you think?

Your name and E-mail address, and of course a comment, are required. I won't do anything evil with your E-mail address.

If you so desire, you may use these tags in your comment: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <s> <strike> <strong>