Make WordPress Core

Opened 4 months ago

Closed 3 months ago

Last modified 3 weeks ago

#61847 closed enhancement (fixed)

Auto Sizes for Lazy-loaded Images

Reported by: mukesh27's profile mukesh27 Owned by: joemcgill's profile joemcgill
Milestone: 6.7 Priority: normal
Severity: normal Version:
Component: Media Keywords: has-patch has-unit-tests needs-dev-note
Focuses: performance Cc:

Description

The purpose of this ticket is to automatically add auto to the beginning of the sizes attribute for any image that is being lazy loaded.

The feature has been implemented in the Enhanced Responsive Images plugin as part of Performance Lab, and it’s currently installed on more than 20k sites.

The feature is standardized in the HTML spec and therefore in a good place to be adopted by WordPress core. It is currently implemented in Chromium (Chrome and Edge) and it is not yet supported in Firefox (see bug #1819581) or Safari (see bug #253143). Because it is a progressive enhancement, there are no adverse effects for browsers that do not support it.

The HTML spec was updated to support sizes="auto" for lazy-loaded images, allowing developers to omit the sizes attribute and have the browser use the image's layout width for resource selection.

@joemcgill put together a quick demo that can be used for observing how the feature is working in supporting and unsupporting browsers.

Implementation:

Currently, it looks like we can apply auto sizes with a fallback (e.g., auto, (max-width: 960px) 100vw, 960px) only when an image has loading="lazy" applied in order to ensure we're not causing a regression for either supporting or non-supporting browsers (see examples). This is expected, but it's nice to see this confirmed by the test cases. An initial implementation will need to be able to account for whether an image has lazy loading applied prior to prepending auto to the sizes value.

Change History (34)

This ticket was mentioned in Slack in #core-performance by joemcgill. View the logs.


4 months ago

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


4 months ago

#3 @mukesh27
4 months ago

  • Owner set to mukesh27
  • Status changed from new to assigned

This ticket was mentioned in Slack in #core-performance by westonruter. View the logs.


4 months ago

#5 @mukesh27
4 months ago

  • Milestone changed from Future Release to 6.7

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


4 months ago

This ticket was mentioned in PR #7253 on WordPress/wordpress-develop by @mukesh27.


4 months ago
#7

  • Keywords has-patch has-unit-tests added; needs-patch removed

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


4 months ago

@flixos90 commented on PR #7253:


4 months ago
#9

@joemcgill Being able to disable auto sizes could be a good idea. Though I would lean against baking this functionality into wp_img_tag_add_loading_optimization_attrs(), as it's not a loading optimization attribute like loading, decoding and fetchpriority are, so semantically that'd be confusing. I'd be wary of making that a "catch-all" function just because it's convenient to implement.

@mukesh27 commented on PR #7253:


4 months ago
#10

@joemcgill Thanks for the feedback. In https://github.com/WordPress/wordpress-develop/pull/7253/commits/deec3ba053e53480807cdcf0fe7b6bfbb55f0797, I introduced a new filter that disables the auto sizes functionality.

I also agree with what @felixarntz mentioned https://github.com/WordPress/wordpress-develop/pull/7253#issuecomment-2319052168.

@flixos90 commented on PR #7253:


4 months ago
#11

@joemcgill @mukeshpanchal27 Given this some further thought, I'll have to revise my take on having a filter.

I'm questioning why we should introduce one to allow disabling auto sizes, for a few reasons:

  • Not everything that WordPress does needs to be modifiable. We shouldn't introduce filters for the sake of having filters. We should ask ourselves whether having a filter makes sense for the concrete functionality.
  • For lazy-loading, the rationale for having a filter is that lazy-loading is a double-edged sword: It is good for sustainability, and it can be good for performance, but if the wrong image is lazy-loaded, it can affect LCP negatively. In other words, overall having it enabled by default makes sense, but there are valid reasons to disable it based on how well it works for a site and, related to that, how much the site owner cares about sustainability vs LCP performance.
  • For the auto keyword on the sizes attribute, I question what such a rationale would be. What are the potential negative side effects of having the keyword present on every lazy-loaded image?
  • On another note, we can always introduce filters later if a need for them is established, but we cannot remove a filter once introduced. This IMO is another reason to introduce filters (and actions) carefully, either based on real use-cases or a anticipating a good reason for it to be needed.

@joemcgill Can you provide a rationale on how a filter to disable auto sizes would help?

@mukesh27 commented on PR #7253:


3 months ago
#12

  • For lazy-loading, the rationale for having a filter is that lazy-loading is a double-edged sword: It is good for sustainability, and it can be good for performance, but if the wrong image is lazy-loaded, it can affect LCP negatively. In other words, overall having it enabled by default makes sense, but there are valid reasons to disable it based on how well it works for a site and, related to that, how much the site owner cares about sustainability vs LCP performance.

The auto value is added to the sizes attribute when images are lazy-loaded. However, if the wrong image is lazy-loaded, the auto value is also incorrectly added to the sizes attribute. We have a filter that updates lazy loading, but it can't remove the auto value from the sizes attribute. In this case, I believe we need a filter that removes the auto value from the sizes attribute. As shown in Joe's example https://github.com/WordPress/performance/issues/791#issuecomment-1856330050, the sizes calculation is worse when an image has only the auto value but is not lazy-loaded.

@flixos90 commented on PR #7253:


3 months ago
#13

The auto value is added to the sizes attribute when images are lazy-loaded. However, if the wrong image is lazy-loaded, the auto value is also incorrectly added to the sizes attribute. We have a filter that updates lazy loading, but it can't remove the auto value from the sizes attribute.

Which lazy-loading filter are you referring to? As far as I'm aware, it's not possible for the logic here to add sizes="auto" to an image that isn't lazy-loaded, so disabling lazy-loading will effectively also disable sizes="auto" addition anyway.

@mukesh27 commented on PR #7253:


3 months ago
#14

Which lazy-loading filter are you referring to?

In quick test i add wp_img_tag_add_loading_attr filter for wp_get_attachment_image() so get auto when the lazy-load attribute is not added.

As far as I'm aware, it's not possible for the logic here to add sizes="auto" to an image that isn't lazy-loaded, so disabling lazy-loading will effectively also disable sizes="auto" addition anyway.

You are right here.

@flixos90 commented on PR #7253:


3 months ago
#15

Which lazy-loading filter are you referring to?

In quick test i add wp_img_tag_add_loading_attr filter for wp_get_attachment_image() so get auto when the lazy-load attribute is not added.

I don't understand. I think that filter only runs for content images, not wp_get_attachment_image(). Can you clarify what you mean?

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


3 months ago

@joemcgill commented on PR #7253:


3 months ago
#17

@joemcgill Can you provide a rationale on how a filter to disable auto sizes would help?

Sure. This functionality introduces an optimization that should work well for a majority of sites—particularly ones that aren't using any plugins or custom code that modifies the way WordPress handles lazy-loading. However, for sites which have a unique setup where this functionality could negatively impacts their site's performance, I think it's good practice to allow sites to easily be able to opt out of these optimizations through the use of a filter, so they can customize how images are served to better fit their specific use case.

In the specific case of auto sizes, if auto is included on an image that is NOT lazy loaded, the current implementation of this feature in Chromium would result in these sizes attributes being calculated at 100vw, rather than the fallback that WordPress provides for browsers that do not yet support auto sizes. Allowing sites that might be impacted by this problem due to third party (or custom) code to opt out of WP's automatic auto sizes implementation would help in those cases.

@flixos90 commented on PR #7253:


3 months ago
#18

@joemcgill

Sure. This functionality introduces an optimization that should work well for a majority of sites—particularly ones that aren't using any plugins or custom code that modifies the way WordPress handles lazy-loading. However, for sites which have a unique setup where this functionality could negatively impacts their site's performance, I think it's good practice to allow sites to easily be able to opt out of these optimizations through the use of a filter, so they can customize how images are served to better fit their specific use case.

In the specific case of auto sizes, if auto is included on an image that is NOT lazy loaded, the current implementation of this feature in Chromium would result in these sizes attributes being calculated at 100vw, rather than the fallback that WordPress provides for browsers that do not yet support auto sizes. Allowing sites that might be impacted by this problem due to third party (or custom) code to opt out of WP's automatic auto sizes implementation would help in those cases.

To me, your points are only theoretical concerns. The logic here only adds sizes="auto" if the image is lazy-loaded, so I don't see what you're saying as a good reason to add a filter to disable it. This would only make sense to me if there was a margin of error in the functionality like there is with lazy-loading - but I don't think there is.

@joemcgill commented on PR #7253:


3 months ago
#19

@felixarntz We've already seen cases in plugins that the Performance Team has written, like Image Prioritizer (related issue), where the loading attribute is modified after all of WP's logic has run, so I don't think this is a theoretical concern, and giving sites a way of opting out of this optimization if needed could help if they instead wanted to implement this in their own way (or not at all).

Given that sites could still work around any issues with this functionality by using filters that run after this is applied, I'm ok with us not adding a filter to disable this functionality initially, but I do generally think it's good practice for us to give easy ways to opt-out of these types of optimization decisions that affect the markup of a site since we can't account for every use case a site might have.

@flixos90 commented on PR #7253:


3 months ago
#20

@joemcgill

We've already seen cases in plugins that the Performance Team has written, like Image Prioritizer (related issue), where the loading attribute is modified after all of WP's logic has run, so I don't think this is a theoretical concern, and giving sites a way of opting out of this optimization if needed could help if they instead wanted to implement this in their own way (or not at all).

That's fair, though the way that this is achieved by that plugin is a (valid) hack via output buffering, so IMO not something core should cater for. I think Core should ensure that anything that is possible via its own APIs (actions and filters) works as expected. You can modify basically anything with things like output buffer, modifying globals, etc., but those aren't encouraged to use, or rather "use at your own risk". So from that perspective I don't find the Image Prioritizer usage relevant for this.

Given that sites could still work around any issues with this functionality by using filters that run after this is applied, I'm ok with us not adding a filter to disable this functionality initially, but I do generally think it's good practice for us to give easy ways to opt-out of these types of optimization decisions that affect the markup of a site since we can't account for every use case a site might have.

Related to your comment in https://github.com/WordPress/wordpress-develop/pull/7253#pullrequestreview-2277887581, I would say that _if_ we introduce a filter, it should be specific to each image (e.g. receive the image tag and context like e.g. the wp_img_tag_add_loading_attr filter works), not a general enable/disable filter. I'd be okay with that because that's more specific to the image being modified and would be suitable for individual modifications. It would still allow completely disabling sizes="auto" (by unconditionally returning false), but that would be neither the primary purpose of the filter nor encouraged, so I think that'd be more reasonable.

My primary concern is about having a filter to generally enable/disable sizes="auto" since, per the above, I don't see any reason that it should be universally disabled (again, different from e.g. lazy-loading where they can be adverse effects from Core not getting it right).

@joemcgill commented on PR #7253:


3 months ago
#21

Totally agree that it would be useful for someone to filter whether this is enabled on based on which specific image the filter was being applied to. However, I think we should go ahead and commit this without a filter for now. We can determine whether a filter is needed and what context will need to be added to that filter after this functionality has been in trunk for a while.

@mukesh27 commented on PR #7253:


3 months ago
#22

I think we should go ahead and commit this without a filter for now. We can determine whether a filter is needed and what context will need to be added to that filter after this functionality has been in trunk for a while.

I agree. In https://github.com/WordPress/wordpress-develop/pull/7253/commits/e4a141a971aa2f51824b20d9e06dc2345a2c497f, I have removed the filter. The PR is now ready for review and commit. 🎉

Should we open a separate Trac ticket for this discussion now, or can we wait and open one in the future if any issues arise?

#23 @flixos90
3 months ago

Pending @joemcgill's review, I think the PR is ready for commit.

Regarding the discussion on introducing a potential filter, in addition to what's already been discussed on the PR, it's worth noting that it's already possible to disable the auto keyword with filters already present, e.g. by doing this:

<?php
add_filter(
        'wp_content_img_tag',
        static function ( $image ) {
                return str_replace( ' sizes="auto, ', ' sizes="', $image );
        }
);
add_filter(
        'wp_get_attachment_image_attributes',
        static function ( $attr ) {
                if ( isset( $attr['sizes'] ) ) {
                        $attr['sizes'] = preg_replace( '/^auto, /', '', $attr['sizes'] );
                }
                return $attr;
        }
);

Given that there's no good reason to generally disable the feature, I think the presence of these two filters is sufficient to alter the behavior if needed. Both of them have the image context available, so they could also alter the presence of auto based on what the image is.

#24 @mukesh27
3 months ago

  • Keywords commit added

The PR got enough approval for commit.

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


3 months ago

#26 @flixos90
3 months ago

  • Owner changed from mukesh27 to joemcgill
  • Status changed from assigned to reviewing

Assigning to @joemcgill for commit per his previous Slack message.

#27 @joemcgill
3 months ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 59008:

Media: Add auto sizes for lazy-loaded images.

This implements the HTML spec for applying auto sizes to lazy-loaded images by prepending auto to the sizes attribute generated by WordPress if the image has a loading attribute set to lazy. For browser that support this HTML spec, the image's size value will be set to the concrete object size of the image. For browsers that don't support the spec, the word "auto" will be ignored when parsing the sizes value.

References:

Props mukesh27, flixos90, joemcgill, westonruter, peterwilsoncc.
Fixes #61847.

This ticket was mentioned in Slack in #core-performance by joemcgill. View the logs.


2 months ago

#30 @flixos90
2 months ago

  • Keywords needs-dev-note added; commit removed

As this is a change relevant to WordPress developers (including related hooks), this should receive a dev note.

I briefly chatted with @joemcgill, and he is already working on one.

#31 @flixos90
4 weeks ago

In 59415:

Media: Avoid images with sizes=auto to be displayed downsized in supporting browsers.

Based on the user agent stylesheet rules outlined in https://html.spec.whatwg.org/multipage/rendering.html#img-contain-size, images that have sizes=auto while applying width: auto or width: fit-content would be constrained to only 300px width.

This changeset overrides said user agent stylesheet rule with a much larger constraint, to avoid the problem.

Additionally, it introduces a filter wp_img_tag_add_auto_sizes which can be used to opt out of the functionality, as an additional measure.

Props joemcgill, flixos90, dooperweb, SirLouen, azaozz, mukesh27, apermo.
Fixes #62413.
See #61847, #62345.

#32 @desrosj
3 weeks ago

In 59435:

Media: Avoid images with sizes=auto to be displayed downsized in supporting browsers.

Based on the user agent stylesheet rules outlined in https://html.spec.whatwg.org/multipage/rendering.html#img-contain-size, images that have sizes=auto while applying width: auto or width: fit-content would be constrained to only 300px width.

This changeset overrides said user agent stylesheet rule with a much larger constraint, to avoid the problem.

Additionally, it introduces a filter wp_img_tag_add_auto_sizes which can be used to opt out of the functionality, as an additional measure.

Reviewed by desrosj, joemcgill.
Merges [59415] to the 6.7 branch.

Props joemcgill, flixos90, dooperweb, SirLouen, azaozz, mukesh27, apermo.
Fixes #62413.
See #61847, #62345.

#33 @daveklep
3 weeks ago

My site’s images are blurry since updating to WordPress 6.7.1. I had not updated to 6.7 but have since found several issues like mine, with solutions way over my understanding level. From what I'm reading here and elsewhere, it seems to be something with the lazy loading or auto sizing (according to other posts), and when I change loading=”lazy” to “eager” in Chrome inspect, the image shows up normal resolution. When I disable –wp–style–global–wide-size and turn it back on in Chrome inspect, the image also shows up normal resolution. While I did create my site, I am not very skilled in web development. I’m not sure what I can do to fix this or if I need someone with that set of skills.

Here is one page as an example: https://ultraviolet.com/uv-c-air-surface-operation/

Last edited 3 weeks ago by daveklep (previous) (diff)

#34 @joemcgill
3 weeks ago

Hi @daveklep, we're currently tracking any follow-up issues related to this feature in #62515. Please feel free to join the discussion there.

Note: See TracTickets for help on using tickets.