Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto Sizes should extend Optimization Detective to add sizes=auto when also lazy-loading images #1099

Open
westonruter opened this issue Mar 28, 2024 · 10 comments
Labels
[Plugin] Auto Sizes Issues for the Auto Sizes plugin [Plugin] Optimization Detective Issues for the Optimization Detective plugin [Type] Enhancement A suggestion for improvement of an existing feature

Comments

@westonruter
Copy link
Member

Feature Description

The Auto Sizes plugin adds sizes=auto to all images with loading=lazy. This is all done server-side without knowledge of which images are actually in the initial viewport across breakpoints. This is knowledge that Optimization Detective has from user visits. In #1077 images which are not in any breakpoint group's initial viewport will be lazy-loaded. However, the addition of loading=lazy to these images should also include sizes=auto. This could be done in the same "Image Loading Optimization" (Image Prioritizer?) dependent plugin of Optimization Detective, or better it could be incorporated as part of the Auto Sizes plugin instead.

@westonruter westonruter added [Plugin] Optimization Detective Issues for the Optimization Detective plugin [Plugin] Auto Sizes Issues for the Auto Sizes plugin [Type] Enhancement A suggestion for improvement of an existing feature labels Mar 28, 2024
@joemcgill
Copy link
Member

Thanks @westonruter. Ideally, I would think that any plugins that are filtering the lazy loading value for images would run prior to the logic that determines whether those images would also make use of auto-sizes. Do you foresee a reason why the processing to add lazy loading from the Optimization Detective would not be able to run before the sizes value is calculated?

@westonruter
Copy link
Member Author

@joemcgill The processing done by Optimization Detective happens after the template has been fully rendered, so long after filters like wp_get_attachment_image_attributes or wp_get_loading_optimization_attributes have run, and after any sizes are calculated for images rendered on the template. Essentially, Optimization Detective should fix up whatever value for the loading attribute was provided during rendering, since server-side it doesn't know in actuality whether the image is going to be in the initial viewport or not. That's only something we can know for sure with the collected URL Metrics. So it may need to add or remove loading=lazy from images, and when it does this it should also add/remove auto from the sizes attribute. Do I understand your question?

@joemcgill
Copy link
Member

You do understand the question, but I don't think I understand the expected usage of the API in that case 😆.

I had assumed that the logic for adding optimizations like lazy-loading, auto-sizes, fetch priority, etc., would be able to be filtered using data collected from the Optimization Detective, rather than needing to add a post-processing step after WP has completed rendering.

@westonruter
Copy link
Member Author

@joemcgill Ah, I see. The optimization needs to be done in the post-processing step because only at that point are we able to see if a given rendered image is the same image that was previously detected, since it relies on matching them with XPaths. Additionally, optimizations can be performed for any arbitrary markup, not depending on filters that WordPress provides during template generation.

@joemcgill
Copy link
Member

Got it. I think the tricky part here is that the auto-sizes plugin will not have any way of knowing whether some other feature is modifying lazy-loading attributes of images after it is finished its processing, which could be true for more than just plugins making use of the Optimization Detective to add post-processing. Can you point to where you are applying this post processing? Perhaps the auto-sizes plugin could preemptively move processing to the same place?

@westonruter
Copy link
Member Author

@joemcgill Yeah, I think in practice it Auto Sizes would continue doing what it is doing today, by injecting auto into the img tags that have loading=lazy at template render time. However, at post-processing time, based on URL Metrics it can add additional sizes=auto to images that also get loading=lazy during post-processing (or else removed when not lazy-loaded). It's similar to what I've proposed for Embed Optimizer in #1054 in which the lazy-embeds should get "unlazied" when post-processing detects that the embed is the LCP. Either that, or when Optimization Detective is running, to do all of the sizes=auto injection during post-processing like you've suggested.

See the current code for the post-processing, although this will be refactored as part of splitting out into Image Prioritizer (#1088).

@joemcgill
Copy link
Member

Ah, I see. You've implemented output buffering as in https://core.trac.wordpress.org/ticket/43258 in order to handle the post-processing. Very cool!

I'm unsure if we should try to simply hook somewhere into the post-processing that OD is doing or if we would be better off trying to support any use cases where images might end up with different lazy-loading attributes based on output buffering. For example, some plugins are likely rendering their own images and applying lazy loading using non-standard ways. It would be great if we could account for those use cases and apply auto-sizes in those cases as well.

@westonruter
Copy link
Member Author

Wouldn't the output buffer post-processing account for any use case, whether images are generated the traditional way or whether they are added in non-standard ways?

@joemcgill
Copy link
Member

Wouldn't the output buffer post-processing account for any use case, whether images are generated the traditional way or whether they are added in non-standard ways?

Yes, OD's output buffering should catch all cases. However, I was thinking about this in a couple of different ways:

If the auto-sizes plugin bundled the OD API, then we could just use that functionality to apply auto-sizes to all lazy loaded images. However, output buffering is not compatible with all setups due to potential conflicts with other plugins that also use an output buffering technique or prematurely flush all buffers.

Alternately, we could integrate with any plugins that are using OD (e.g. the future Image, by adding a callback to one of ODs hooks to do some post-processing cleanup. However, this would only fix compatibility issues introduced by ODs post-processing and not any other plugins that are doing similar things to add lazy-loading after WP's default image processing is done.

Given that the purpose of auto-sizes is to test the feature prior to potentially adding it to Core, I'm a bit wary of making OD a dependency of the feature and would prefer to build it in such a way that supports a wide variety of plugins.

@westonruter
Copy link
Member Author

If the auto-sizes plugin bundled the OD API, then we could just use that functionality to apply auto-sizes to all lazy loaded images. However, output buffering is not compatible with all setups due to potential conflicts with other plugins that also use an output buffering technique or prematurely flush all buffers.

Do you have any example of this? I've heard this raised before as a problem, but I haven't seen it in actual plugins. Multiple plugins should be able to do output buffering concurrently. For example, page caching plugins rely on output buffering and they work with the AMP plugin that also does output buffering.

Given that the purpose of auto-sizes is to test the feature prior to potentially adding it to Core, I'm a bit wary of making OD a dependency of the feature and would prefer to build it in such a way that supports a wide variety of plugins.

Yeah, I didn't mean to make it a dependency. But rather to leverage it if it is installed. Similarly to Embed Optimizer leveraging Optimization Detective if it is installed to prevent lazy-loading LCP embeds, but otherwise continue to lazy-load all embeds using traditional WP filters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Plugin] Auto Sizes Issues for the Auto Sizes plugin [Plugin] Optimization Detective Issues for the Optimization Detective plugin [Type] Enhancement A suggestion for improvement of an existing feature
Projects
Status: Not Started/Backlog 📆
Development

No branches or pull requests

2 participants