Add image preload option to help boost actual and perceived performance

Vitalii Podoba
We can see a MR !3178 merge error.
is the issue still persists.

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

rerolling the patch respective to the 10.1.x along with the reroll.

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

Since this is changing the configuration it will need an upgrade path for existing sites.

Also will need a change record.

@HitchShock thanks for all the work on this!

Even though the default is FALSE if I go into the field formatter, make no change and click save. If I do config export and it changes the config it will need an upgrade path.

Removing credit for #14 as it's expected to check a patch before uploading.
You can check for build errors make sure to run ./core/scripts/dev/commit-code-check.sh before uploading a patch to make sure there are no issues with code formatting. see https://www.drupal.org/docs/develop/development-tools/running-core-devel...

upgrade looks good.

Can mark it after the change record.

Unpublished the CR, because when it is published this is announced trough rss and that should only be done when the issue is committed.

Will this be the same functionality as provided by https://www.drupal.org/project/responsive_image_preload ?

There are a lot of tests updated with '#image_preload' => FALSE,, does this mean that all custom/contrib code will have to do that as well? Can we not rely on the default value instead? I'd love to see less changes in the tests for this PR.

For the new open threads in the MR

How is the work here vs in #3048458: Add support of "preload" attribute into FileMediaFormatterBase different? It isn't clear.

Will also post into #ux channel for feedback.

Posted in #ux channel and it hasn't had a full review but here is comment from @rkoller who was nice enough to take a quick look.

i don’t have much time to look in depth but i sort of agree with lucas heddn. it feels odd having the preload checkbox outside the image loading field set. i dont know the technical details and why it may be semantically more precise to separate both according to hitchshock. but from a users perspective preload and lazy load are both in the context of loading the image. and as lucas said it is advanced functionality.
currently you have the preload option exposed outside the collapsed fieldset. and users tend not to read lengthy descriptions. therefore it might happen that exactly that happens what the description advices not to do. people clicking the checkbox.
and yes i agree the description is long and hard to read. havent posted a comment on the issue cuz i havent come up with an alternative draft yet.
but one detail. …because not everything are critical… sounds grammatically wrong. shouldn’t that be is critical?

So seems like there is more votes leaning towards moving this into the image loading fieldset.
Will need a shorter description (if possible)
And nit picky "everything are critical" should be updated.

@HitchShock:

Good timing! @Andrew_B and I are looking at this issue at DrupalCon Pittsburgh today. We will try to give a UX review today.

Tested 3309016-add-image-preload branch using DrupalPod in core version 10.1.x

Viewed UI changes through the image field present on the Article content type (Structure → Content types → Article → Manage display).
The UI here looks great; Expanding the image loading dropdown in format reveals the "Preload" checkbox above the Image loading attribute radio. Hiding the attributes section upon checking "Preload" clearly indicates the other attribute options are rejected.

Compared page source before and after updating the image field format with preload active to verify that the respective link tags were placed in HEAD with rel="preload" as="image". Everything appears as described.

Working great! Very useful enhancement to the image field formatter.

Overall this looks good, added some comments on the MR though. It's still tagged for usability review, has that been done?

Usability review

I do not have an opinion on the implementation of this feature, or whether it is useful enough to be added to Drupal core. (If I understand correctly, this feature is already provided by the blazy contrib module. If only a few sites need the feature, then that should be where it stays.) I am just reviewing the usability of the configuration form.

The current MR introduces a very unusual pattern. By default, the Preload option is un-checked and the "Image loading attribute" radio buttons are visible. If Preload is selected, then the radio buttons are hidden. The usual "progressive disclosure" pattern is different: initially, some options are hidden, keeping the form simple, but they appear if some other non-default option is selected.

If I read the code correctly, then the choice of loading attribute is ignored when Preload is selected:

  if ($image_src !== '' && $variables['image_preload']) {
    // Make sure that we don't have loading attribute when we use preload.
    unset($variables['attributes']['loading']);
    // ...
  }

I think it makes more sense to have three radio buttons. Putting Preload third makes sense: then the options are ordered by how aggressive we are about loading. We will have to change the label "Image loading attribute" since one of the choices does not set the loading attribute.

Here is a first draft:

Lazy render images with native image loading attribute (loading="lazy"). This improves performance by allowing browsers to lazily load images.

Image loading

• Lazy (loading="lazy")
  
  Delays loading the image until that section of the page is visible in the browser. When in doubt, lazy loading is recommended.
• Eager (loading="eager")
  
  Force browsers to download an image as soon as possible. This is the browser default for legacy reasons. Only use this option when the image is always expected to render.
• Preload (rel="preload")
  
  Preload to optimize the loading of late-discovered resources. Normally large or hero images below the fold.

Select how images are loaded. Learn more about the loading attribute for images and the preload attribute for links.

I do not love having descriptive text in three places (above the fieldset, after each option, and at the end of the fieldset). But I think that reorganizing it is out of scope for this issue.

The labels I suggest are imprecise because they do not indicate that the loading and rel attributes apply to different HTML elements. But I want to keep the labels short, and the link text at the bottom suggests what is going on.

With regard to placement of the preload option in the UI, it doesn't make sense to include preload with the eager and lazy options. To @hitchshock's point, preload is a separately rendered link tag. It can have a big impact of page performance and should be in core.

I agree that preload should be kept separately from eager/lazy because preload adds a new HTML tag to the HEAD, as opposed to an attribute on the IMG tag. It is not an alternative option to eager/lazy but an additional feature to enhance LCP optimization even more than what eager can do.

Also, with Lighthouse now in the process of switching over to use "insights" instead of legacy "audits", there will also be forward momentum on the sister issue #3366828: Add option for fetchpriority attribute within "image loading" area on image field formatter, which uses a similar checkbox toggle in the image field formatter UI. So when evaluating UI patterns it would be good to take into account that an ideal "final" product here would have two additional opt-in checkboxes, one for Preload and another for Fetch Priority.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

In a slack thread about introducing early hints for images, we identified with @catch a potential serious issue.

The approach for the D11 MR adds a preload for every image in a responsive image source set. This seems counter-productive for preload. Ideally, we'd want to preload just the one variant needed, but it is fundamentally impossible to pick which one from the server-, because it depends on client-side information (viewport size, rendered layout).

(Sidenote: the D10 MR does not have the responsive image work).

https://drupal.slack.com/archives/C0D5GJZ8B/p1777304794431579

adds a preload for every image in the set [...] there’s no way to solve this differently on the server side. [...] I don’t see any serious issue here.

I cant speak for @catch but what I, for sure, was thinking was that preloading several copies of an above-the-fold hero image, eg a super large (eg 2400px wide for large screen retina display), large (desktop), medium (tablet), and small (mobile), all on a mobile device, in that order because that is the presumably the order used to create the <picture> element in responsive images, is precisely the kind of problem that I would consider a serious waste of mobile bandwidth.

Do you have any proof that the images would not start automatically downloading because they're supposed to be used as part of a media query that doesn't match mobile?

https://www.drupal.org/project/normalized_responsive_images is an up and coming recipe that offers up to 20 image sizes. I would push back on 3 or 4. https://github.com/ausi/respimagelint is fairly well known linter and recommends many images sizes.

Yes preloading that many images would really undermine the point of reponsive images.

Preload allows for srcset + sizes on the link https://web.dev/articles/preload-responsive-images - that allows the browser to choose (essentially guess) which image might get used. But it also conflicts with sizes=auto which is newer and preferable, and relies on the page rendering enough to know how much space there is for the images - which implies not preloading at all.

https://web.dev/articles/preload-responsive-images#picture seems to suggest, but it isn't clear to me if this is proposed or actual... that we can add preload in the header if we also add media queries.

Thank you @hitchshock for providing proof, and screenshots.

So the bandwidth concern raised in #50 and #52 appears nuanced: it's not a problem on smaller viewports, but it becomes problematic on larger screens. This is due to how the media queries are interpreted: classic "mobile first"-style min-width conditions are cumulative, so multiple sources can match simultaneously unless explicitly bounded by max-width.

Conversely, implementations using only max-width breakpoints would invert the problem, causing all variants to preload on smaller screens.

This behavior differs from typical responsive CSS expectations, where cumulative breakpoints are very common and generally safe. For image preloading, however, non-exclusive conditions can lead to unintended concurrent downloads.

This should be explicitly documented, with clear guidance that breakpoints must be mutually exclusive when used with preload. It would also be worth highlighting this caveat in the UI, since most developers won’t think to verify actual network behavior per breakpoint. Maybe even we should provide a footgun warning, whereby we test breakpoints configured in responsive image sets to ensure they use the exclusionary methodology, and warn users when they're incorrectly configured.

We should also write an explicit test that shows parity between media queries on the preload and the responsive image set breakpoints used.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

The Needs Review Queue Bot tested this issue. The merge request has merge conflicts and cannot be merged. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Code changes LGTM, nice work!

Tested with the demo_umami profile on a desktop viewport with a retina display.

1. Edit the Scale Crop 7:3 Large media view mode, used by the Umami hero banner block. Enable preload on the image field formatter using the Hero responsive image style. This triggers the breakpoint warning.
2. Edit the Responsive 3:2 media view mode, used by the Umami frontpage cards via the card_common entity view mode. Enable preload on the image field formatter using the 3:2 Image responsive image style. This does not trigger the warning.
3. Load the Umami homepage with the DevTools Network panel open:
   • Hero: two image styles are downloaded: wide (desired) and tiny (undesired).
   • Cards: two image styles are downloaded: large_3_2_2x (desired) and square_large (undesired).

   Side note: repeated homepage loads while logged in as admin did not immediately add the expected preload links to the DOM. After clicking through various admin pages, waiting, and rebuilding caches, both eventually appeared. I'm not certain what fixed this, but it may be out of scope for this issue. Tested on coder.ddev.com.

IMO it makes sense to slightly widen the scope here: get Umami working as expected, and use it to demonstrate how the preload setting should work.

For the Hero responsive image style, I suggest updating the breakpoint configuration so it does not trigger the warning and does not download two images at desktop sizes. I suspect the tiny breakpoint needs to be made exclusionary, but that needs a deeper look. Once the breakpoint stack is corrected, we should probably add both a failing test for the current broken configuration and a passing test for the corrected one.

For 3:2 Image, preloading the homepage cards is not realistic, since below-the-fold cards should remain lazy-loaded. However, the recipe detail page also uses the Responsive 3:2 media view mode, where the image appears above the fold and could benefit from preload. The complication is that the same media view mode is used in both contexts, so I’m not sure whether it can be configured as lazy-loaded in one place and preloaded in another without a preprocess-level alteration.

It probably still makes sense to exercise the 3:2 Image code path, since it uses viewport sizing instead of breakpoints. We need to determine why two images are loaded there too, and whether that can be tested by generalizing hasNonExclusiveBreakpoints().

I'm afraid this PR getting very large. We might want to split it up into 3 smaller pieces. One for responsive image, one for media and one for image.module. At a git stat of 68 files +1930 −55, this is getting hard to review. Splitting it up will give us time to learn as we go too. And if you break the responsive image stuffs out, it punts on the UX concerns around multiple files being downloaded until later.

When we first introduced loading attribute support into core, we also brought it in over multiple PRs for similar review reasons. You might be able to keep media and image in the same PR, but splitting things up is almost always a good thing.

I didn't dwell on it a lot in my initial review but the UX concerns with responsive image are sufficient to need us to spend some time on the UX wording of things. Which reminds me, do we need to do a UX review of the text for adding preload to field formatters? Has that happened yet? A formal review would be nice but even a screenshot posted here and some informal reviews would be great. There seems to be enough nuances here around best practices and suggestions about _not_ using preload in certain situations that guidance would help.