twentysixteen icon indicating copy to clipboard operation
twentysixteen copied to clipboard

twentysixteen_content_image_sizes_attr uses first parameter incorrectly

Open smerriman opened this issue 8 years ago • 3 comments

The first line of this function uses:

$width = $size[0];

Despite the fact all uses of this filter in core pass $size as a width/height array, it is incorrect to assume $size is an array, as it may be a size name. My preference is that $size should be enforced to be an array, but this was rejected as wontfix:

https://core.trac.wordpress.org/ticket/37479

This means you need to duplicate the code in the core wp_calculate_image_sizes function to determine if $size is an array or string and pull out the correct width.

smerriman avatar Oct 10 '16 21:10 smerriman

Despite the fact all uses of this filter in core pass $size as a width/height array

twentysixteen_content_image_sizes_attr only interacts with core. It's not meant really to be used standalone. You could use it that way if you wanted, and the documentation in functions.php explains that the sizes parameter should be an array

tevko avatar Oct 10 '16 21:10 tevko

No, it's applied as a filter, and therefore interacts with every single call made to wp_calculate_image_sizes.

The point made by joemcgill was that plugin authors may call the wp_calculate_image_sizes passing a size name. The filter in twentysixteen would then break.

I don't agree with joemcgill's point, but one of you must be wrong :)

smerriman avatar Oct 11 '16 00:10 smerriman

No, it's applied as a filter, and therefore interacts with every single call made to wp_calculate_image_sizes.

My mistake, I meant to imply that but I was clearly not specific enough! I see what you're talking about in reference to plugin authors, and how that can cause broken code in the theme.

Joe and I both worked on getting responsive images into core, and I wrote the specific code that you're referencing into the twentysixteen theme.

I'm more than happy to submit a PR that fixes this issue since core will not enforce an array as the only valid parameter,

tevko avatar Oct 11 '16 01:10 tevko