[wp-trac] [WordPress Trac] #41895: wp_calculate_image_srcset filter: Improve the documentation for, or rename, this filter so it's clear it should work on an array.

[WordPress Trac]

#41895: wp_calculate_image_srcset filter: Improve the documentation for, or rename,
this filter so it's clear it should work on an array.
-------------------------+-----------------------------
 Reporter:  johnnyb      |      Owner:
     Type:  enhancement  |     Status:  new
 Priority:  normal       |  Milestone:  Awaiting Review
Component:  Media        |    Version:  trunk
 Severity:  normal       |   Keywords:
  Focuses:               |
-------------------------+-----------------------------
 = 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,
 [https://core.trac.wordpress.org/browser/tags/4.8.1/src/wp-
 includes/media.php#L1203 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
 [https://developer.mozilla.org/en-US/docs/Web/HTML/Element/img 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
 [https://www.facebook.com/groups/advancedwp/ Advanced WP Facebook Group] a
 prominent member of the WP community
 [https://www.facebook.com/groups/advancedwp/permalink/1624922314236643/?comment_id=1625126477549560&reply_comment_id=1625763707485837&notif_t=group_comment_reply&notif_id=1505579798429304
 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.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/41895>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform