Add image attributes and additional context to the wp_calculate_image_srcset and wp_calculate_image_sizes filters

[kylereicks]

Because the srcset and sizes attributes try to give the browser guidance on how CSS is going to handle images, we should be able to take into account image class attributes (and other attributes that could be used as CSS selectors) when using the wp_calculate_image_srcset and wp_calculate_image_sizes filters to set the appropriate srcset and sizes.

It would also be helpful to be able to supply a context to functions like wp_image_add_srcset_and_sizes and wp_make_content_images_responsive so we can calculate different srcset and sizes attributes if a medium sized image is in the main content section vs. in an aside or other section.

[kylereicks]

Adds image attributes and additional context to wp_calculate_image_srcset and wp_calculate_image_sizes filters

[kylereicks]

As an example, if a theme had a multi column layout like the following:

img, figure{
	max-width: 100%;
	height: auto;
}
.alignright{
	max-width: calc(100% + 3em);
	float: right;
	margin-right: -6em;
}
.entry-content{
	max-width: 350px;
	margin: 0 auto;
	padding: 1em 0;
	columns: 1;
	column-gap: 12em;
}
@media screen and (min-width: 600px){
	.alignright{
		max-width: 150%;
		margin-right: -50%;
	}
}
@media screen and (min-width: 900px){
	.alignright{
		max-width: calc(100% + 6em);
		float: right;
		margin-right: -6em;
	}
	.entry-content{
		max-width: none;
		columns: 2;
		margin: 0 6em;
	}
}
@media screen and (min-width: 1200px){
	.entry-content{
		columns: 3;
		column-gap: 6em;
	}
	.alignright{
		max-width: calc(100% + 3em);
		float: right;
		margin-right: -3em;
	}

}

An image with no alignment class might have a sizes attribute like (min-width: 1200px) calc((100vw / 3) - 3em), (min-width: 900px) calc((100vw / 2) - 6em), (min-width: 350px) 350px, 100vw whereas an image with the class .alignright might need a slightly different sizes attribute a more like (min-width: 1200px) calc(100vw / 3), (min-width: 900px) calc(100vw / 2), (min-width: 600px) 525px, (min-width: 350px) calc(350px + 3em), 100vw

Without access to the image attributes (class, id etc.) when filtering srcset and sizes, we cannot tell the difference between a medium sized image and a medium sized image with the class .alignright even though they may be sized differently at some breakpoints.

[herrschuessler]

I second @kylereicks considerations. There are many use cases where responsive images will need a different sizes attribute based on their alignment value, e.g. when floated images are displayed smaller than non-aligned, full-column images. Therefore, the alignment value should be available in wp_calculate_image_sizes.

[greatestview]

+1

Currently there is no way to tell exactly, which image is small and wide in wp_calculate_image_sizes. It’s the same with wp_get_attachment_image_attributes. I would appreciate providing the classes of that image as an additional argument, or maybe better, the whole image html code.

[adamsilverstein]

@kylereicks Thanks for your ticket and patch and welcome to trac!

Overall the patch looks good and the use case makes sense... this seems like a valuable addition to the filter. Can you give an example of how you would use the context variable, or why you added that?

It would be good to add some unit tests that demonstrate how filter change is used: how the patch enables altering the sizes based on both attr values and context.

[kylereicks]

Update patch to remove the "context" attribute. Add a test to make sure that srcsets and sizes can be successfully updated via filters.

[kylereicks]

I added a new patch to remove the "context" attribute. It may not end up being useful for most cases and is not a critical addition. The new patch also adds unit tests to confirm that the wp_calculate_image_srcset and wp_calculate_image_sizes filters can access an image's class attribute and update their values based on that attribute.

[strarsis]

Currently, I have to resort to either the ratio (when hard crop is used for that particular image size) or other tricks to deduce the usage of a particular image in the wp_calculate_image_sizes filter.
Basically the usage context of the image has to be "encoded" as image size dimensions or ratio, there are no other means. Just knowing the markup/Gutenberg block the image is used in would help tremendously.

[adamsilverstein]

Revisiting tickets and this still seems useful. Marking as 5.7 as a reminder to land this.

[adamsilverstein]

@kylereicks I couldn't quite understand the regex your patch includes to extract the image attributes. Can we just not pass attributes in that case? Developers still get the source and could extract attributes if they need them.

If we do want to try to extract the attributes, I would ask:

• Document the RegEx part by part so it can be understood more easily.
• Move the attribute extraction into a helper.
• Add tests that validate the helper works correctly in all cases.

Otherwise this patch looks good: when we have the image attributes, we pass them to the wp_calculate_image_sizes and wp_calculate_image_srcset filters to make those filters more useful for developers.

[adamsilverstein]

36982.diff​ is a refresh against trunk with the regex bit removed. I resolved several merge conflicts so this deserves careful review.

[Mista-Flo]

Patch looks good, well done Adam

Trac ticket: https://core.trac.wordpress.org/ticket/36982

[adamsilverstein]

In 36982.2.diff​ I added back a missing line from resolving the merge conflicts.

Pushed current state to PR to run all tests.

[adamsilverstein]

36982.2.diff​ is a refresh against trunk. Rerunning all tests on the PR.

Trac ticket: https://core.trac.wordpress.org/ticket/36982

[Mista-Flo]

Thanks for the refresh Adam, the removed line came back, code looks good, unit tests are pretty solid, I think it's ok to go.

[antpb]

I think the PR is false green because the changes are different from the uploaded patch.

I'm seeing test failures locally around undefined $image_attr attributes. Going to investigate fixing these on my end.

example failure
57) Tests_XMLRPC_wp_newPost::test_post_thumbnail Undefined variable: image_attr

I suspect it's a quick find/replace sort of fix for these tests.

Would appreciate a review before commit!

[adamsilverstein]

Yea, @antpb apologies that is the wrong PR linked, let me find the right one :)

[adamsilverstein]

This is the correct PR where the tests are currently running after I pushed an updated with fixes for phpcs: ​https://github.com/WordPress/wordpress-develop/pull/968.

[adamsilverstein]

36982.4.diff​ also corrects phpcs warnings.

[adamsilverstein]

I'm seeing test failures locally around undefined $image_attr attributes. Going to investigate fixing these on my end.

I see these as well in GitHub, thanks for investigating.

[antpb]

Just investigating the first failure around the get_custom_logo()
1) Tests_WP_Customize_Manager::test_import_theme_starter_content Undefined variable: image_attr

I tested the function on a fresh install with a custom logo set and am seeing errors when using the function.

Getting the notice
Notice: Undefined variable: image_attr in /var/www/src/wp-includes/media.php on line 1529

I need to dig deeper into the other ~50 failures to see if theres another function we'll need to factor this fix into. Likely a repeated function through our tests. get_custom_logo doesnt seem to be more than a 5 of them.

[adamsilverstein]

I see the issue, we are using $image_attr in wp_calculate_image_sizes without actually passing it in. Checking history to see where that was lost.

[adamsilverstein]

36982.5.diff​ fixes the missing $image_attr issue in wp_calculate_image_sizes.

[adamsilverstein]

Still seeing some failing tests, these are the tests added in the patch. I'll have to dig in further to see exactly why they are failing.

Hey @kylereicks, if you are available I would appreciate your help getting this over the finish line!

[hellofromTonya]

5.7 Beta 1 is happening in a few minutes. Ran out of time to get this ticket into the release. Punting to 5.8.

If any maintainer or committer feels this can be resolved in time, or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

[adamsilverstein]

5.7 Beta 1 is happening in a few minutes. Ran out of time to get this ticket into the release. Punting to 5.8.

Thanks @hellofromTonya - I'll pick this up again for 5.8.

[desrosj]

Today is 5.8 feature freeze. Since there has been no progress this release cycle, I'm going to punt to Future Release. When someone is able to tackle this, it can be moved back to a numbered milestone.

[adamsilverstein]

Trying to get tests to pass I noticed two issues:

• Test string is missing loading="lazy" (added since patch)
• Sizes are not filtered correctly in because calls to wp_calculate_image_sizes in wp_image_add_srcset_and_sizes do not pass the image attributes, so checks like looking for the class in the test filters fail.

I think a little more work is needed to make the filters work properly there with attributes as well if we want to consider this complete. @kylereicks, I know it has been a long time since you worked on this, any chance you want to pick it up again?

[adamsilverstein]

36982.6.diff​ is a refresh against trunk and adds loading="lazy" to the expected image test string. Tests still not passing.

[antpb]

Thanks for the refresh Adam! I think this one may be a little too close to 5.9 but I'm optimistic the next release is a good target.

If anyone wants to move this back into 5.9 please feel free.