wp_calculate_image_srcset filter: Improve the documentation for, or rename, this filter so it's clear it should work on an array.

[johnnyb]

The Problem

Despite having the same name as the wp_calculate_image_srcset() function and being inside of that function the wp_calculate_image_srcset filter, here in the current release, does not directly modify the output of the function as convention would dictate. This leads to confusion, so theme and plugin developers do things that lead to bugs.

The wp_calculate_image_srcset filter filters the $sources variable, which is an array of arrays, each containing information about one of the image sources that WP has decided to add to the srcset. However the wp_calculate_image_srcset() function returns either a string HTML ​srcset attribute for use on an img tag or false if there's only one source or some other failure.

Because the wp_calculate_image_srcset() function can return false, developers assume that wp_calculate_image_srcset filters can return false as well, thus converting the $sources array into a boolean false. This causes a problem when a second plugin or theme also tries to filter wp_calculate_image_srcset and tries to loop over the values in $sources, (foreach( false) causes a PHP warning).

Real-World Examples

This is happening in the real world: If you're hosted on WPEngine, and use Themeco's X or Pro theme, the PHP warning pops up in some of your pages, (especially in WPEngine's staging environment). This is because X and Pro use wp_calculate_image_srcset to change $sources to false, then WPEngine's Must-Use plugin tries to iterate over $sources.

When contacted about the problem, (with the suggestion they return an empty array), Themeco's response was "In looking over WordPress' official documentation for that function/hook, I believe that boolean false should be the correct value to return" with a link to the documentation for the function.

When the issue was raised in the the ​Advanced WP Facebook Group a prominent member of the WP community ​appears to have made the same logical jump, that the filter filters the output of the function, and wouldn't listen to any further discussion.

Possible suggestions to improve the situation

1. If we're ok with setting $sources to false, the docblock for the wp_calculate_image_srcset filter should be changed to indicate that the $sources variable being passed to a filter may not be a variable.

2. If we're not ok with setting $sources to false, maybe we should add a _doing_it_wrong() if $sources type is changed from an array.

3. Whatever we do, we should document an expected return type in the docblock for the filter.

3. Since the filter doesn't actually filter the output of the function, the filter name could be changed to something like wp_calculate_image_srcset_sources. I know this is a breaking change, which may never happen because it's a breaking change, but it would be the best fix, if breaking changes can be dealt with.

[Otto42]

+1

I agree that this is confusing. I made the same assumption that the filter named the same as the function would filter its output, but it's filtering something else here instead.

This filter should be renamed to something else, and the original filter should be changed to only filter the final output. That way, any current users of this filter who are returning false for it will have the expected result instead of a potentially incorrect one.