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

Image Prioritizer fails to preload LCP images for picture elements #1312

Open
westonruter opened this issue Jun 22, 2024 · 4 comments
Open
Labels
[Plugin] Image Prioritizer Issues for the Image Prioritizer plugin (dependent on Optimization Detective) [Plugin] Modern Image Formats Issues for the Modern Image Formats plugin (formerly WebP Uploads) [Plugin] Optimization Detective Issues for the Optimization Detective plugin [Type] Bug An existing feature is broken

Comments

@westonruter
Copy link
Member

westonruter commented Jun 22, 2024

The Image Prioritizer plugin adds a preload link tag with fetchpriority=high for the URL contained in the src attribute of the img. When the Modern Image Formats plugin is active and the "Use <picture> element" option is enabled, then the img is wrapped in picture with other source elements for WebP and AVIF image formats. Because of this, the preload link will usually be wrong because the actual URL being used for the LCP image will be the WebP or AVIF image, not the JPEG fallback.

For example, a page containing this LCP element:

<picture class="wp-picture-13">
  <source type="image/webp" srcset="http://localhost:8888/wp-content/uploads/2024/06/bison1-1024x668-jpg.webp 1024w, http://localhost:8888/wp-content/uploads/2024/06/bison1-300x196-jpg.webp 300w, http://localhost:8888/wp-content/uploads/2024/06/bison1-768x501-jpg.webp 768w" sizes="(max-width: 1024px) 100vw, 1024px">
  <source type="image/jpeg" srcset="http://localhost:8888/wp-content/uploads/2024/06/bison1-1024x668.jpg 1024w, http://localhost:8888/wp-content/uploads/2024/06/bison1-300x196.jpg 300w, http://localhost:8888/wp-content/uploads/2024/06/bison1-768x501.jpg 768w" sizes="(max-width: 1024px) 100vw, 1024px">
  <img fetchpriority="high" width="1024" height="668" src="http://localhost:8888/wp-content/uploads/2024/06/bison1.jpg" class="attachment-1024x668 size-1024x668 not-transparent" alt="" data-has-transparency="false" data-dominant-color="8c7d58" style="width:100%;height:65.23%;max-width:1024px;--dominant-color: #8c7d58;">
</picture>

Is getting this preload link:

<link data-od-added-tag="" rel="preload" fetchpriority="high" as="image" href="http://localhost:8888/wp-content/uploads/2024/06/bison1.jpg" media="screen">

In this specific example, the LCP element is the same across all breakpoints, so the fetchpriority=high on the img itself is sufficient and the preload link is not required. Nevertheless, if the LCP element is not the same across all breakpoints, the fetchpriority=high attribute cannot be added to the img or else it will get loaded with high priority for some breakpoints for which it is not the LCP image. This basically uncovers a performance problem with using <picture> as it cannot be reliably loaded with high priority to improve LCP. This is essentially a known issue per GoogleChrome/web.dev#6150.

For Image Prioritizer, the preload link needs to be omitted when the related LCP element is a picture element. It can still add fetchpriority=high to the img as it does now when all breakpoints share it as the LCP element.

Cheers to @aaemnnosttv for talking this through with me at WCEU.

@westonruter westonruter added [Type] Bug An existing feature is broken [Plugin] Modern Image Formats Issues for the Modern Image Formats plugin (formerly WebP Uploads) [Plugin] Optimization Detective Issues for the Optimization Detective plugin [Plugin] Image Prioritizer Issues for the Image Prioritizer plugin (dependent on Optimization Detective) labels Jun 22, 2024
@westonruter
Copy link
Member Author

As came up in the Core Performance chat today, will a browser skip preloading a link that has a type for an image format that it doesn't support? If so, then we could add the preload link for AVIF or WebP. But we would still have to omit the preload link for the JPEG fallback, since then even browsers browsers that support AVIF/WebP would be fetching the JPEG as well even though only the former would be used.

@westonruter
Copy link
Member Author

It looks like Chrome at least does exclude preloading image formats that it doesn't support: https://jpegxl-preload-link.glitch.me/

This is in fact part of the HTML spec:

4.2.4.2 Processing the type attribute

If the type attribute is present, then the user agent must assume that the resource is of the given type (even if that is not a valid MIME type string, e.g. the empty string). If the attribute is omitted, but the external resource link type has a default type defined, then the user agent must assume that the resource is of that type. If the UA does not support the given MIME type for the given link relationship, then the UA should not fetch and process the linked resource; if the UA does support the given MIME type for the given link relationship, then the UA should fetch and process the linked resource at the appropriate time as specified for the external resource link's particular type. If the attribute is omitted, and the external resource link type does not have a default type defined, but the user agent would fetch and process the linked resource if the type was known and supported, then the user agent should fetch and process the linked resource under the assumption that it will be supported.

So it looks like we can indeed add preload links for picture elements, but only for the AVIF/WebP source and not for the fallback JPEG. If the source also has a media attribute, then this will need to be merged with whatever media is computed for the LCP element viewport group.

@westonruter
Copy link
Member Author

If a picture element is used not merely as a way to serve a modern format but instead has multiple source elements for art-directed images, then there will need to be separate preload links for each one, again replicating the media attribute from the source element to the link element (and merging with the viewport group media query).

@westonruter westonruter added this to the image-prioritizer n.e.x.t milestone Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Plugin] Image Prioritizer Issues for the Image Prioritizer plugin (dependent on Optimization Detective) [Plugin] Modern Image Formats Issues for the Modern Image Formats plugin (formerly WebP Uploads) [Plugin] Optimization Detective Issues for the Optimization Detective plugin [Type] Bug An existing feature is broken
1 participant