Leverage the 'loading' html attribute to enable lazy-load by default for images in Drupal core

Here is the first attempt on getting the loading attribute added to the majority of the images rendered by Drupal Core.
Check the commit here.

Working on tests.

Added a simple test coverage for this new feature.

IMHO I think we shouldn't implement lazy loading for the themes logos since they usually load at first neither add a test coverage that prove that.

Looks ready for review, test added

Works nicely on regular media fields, but is it out of scope to add also to core ckeditor?
Maybe we should document why width/height is checked?
Following is taken from https://web.dev/native-lazy-loading/ Without dimensions specified, layout shifts can occur, which are more noticeable on pages that take some time to load.

@sokru Yep ckeditor support is something we want. Checking that.

Added ckeditor support.

Fixing tests.

Updated ImageDimensions test asserts

Fixing tests.

Update EditorPrivateFileReferenceFilterTest test to use a real image to get getimagesize function happy.

@heddn Thanks for checking it.

Working on it.

Worked on #20.

Passing on a few comments from folks on the technical team at Google who don't yet have accounts here on Drupal.org:

Felix@Google: One technical comment, for consideration: I wonder whether the modification of attributes in https://git.drupalcode.org/project/drupal/-/merge_requests/6/diffs#f8bdc... should be more "defensive"? As in:

1. Only set width and height if neither of them are already set on the image.
2. Only set loading if it is not set on the image and if both width and height are set (either because of 1. or because they were already set previously).

Maybe there's no circumstance where that would be the case, but just pointing it out.

And then unrelatedly:

Scott@Google and Ben@Google: Do we want to consider applying the same loading="lazy" defaults when iframes are included? They are the other common case besides images where that attribute can be valuable. But maybe it doesn't make sense to assume a default for users in that case.

Addressing #23 and #24 feedback.

If the loading="lazy" is hard-coded in the theme function, a site builder gets no say in things. How do we set loading="auto" or loading="eager"?

At the least, shouldn't lazy loading be a configurable option on image field formatters and the CKEditor tool?

Hi there, I just got aware of this initiative. I don't have much time to read this whole thread right now, but I have to mention that 2 months ago I started a little module that tries to do that: https://www.drupal.org/project/native_lazy_loading
Anyway, I'll have a closer look at this issue later, as it looks like there's some sort of effort duplication.

Thank you all for your work on this useful initiative. I added some comments on git.drupalcode.org.

Some more general thoughts I had by comparing the core patch and the humble contrib module I implemented:

1. It looks like width and height tags are added only when using the HTML editor (CKEditor): what if my image is in an image field?
2. Are responsive images handled? It has to be mentioned that adding height and width tags on them may change the displayed size if the CSS tells nothing about it (unlikely on a themed website), because they will take precedence over the sizes attribute. This can disturb some users.
3. Are image styles (and responsive image styles) correctly handled? On my project I got some feedback about styled images tagged with original height and width.

I have to agree with @andrewmacpherson here (#26). I think it is important to allow for some images to be configured when loading="eager" is appropriate. I recently implemented a primitive version of this on Chromatic's website wherein we used an image style's ID to conditionally set loading="eager" in certain cases. Here is some sample code showing we pulled this off:

<?php

/**
 * @file
 * Module to support native browser lazy loading of images.
 */

/**
 * Implements hook_preprocess_image().
 */
function chromatic_lazyload_preprocess_image(&$variables) {
  // If `loading` attribute is already set, do nothing, otherwise set to `lazy`.
  if (!isset($variables['attributes']['loading'])) {
    $variables['attributes']['loading'] = 'lazy';
  }

}

/**
 * Implements template_preprocess_responsive_image().
 */
function chromatic_lazyload_preprocess_responsive_image(&$variables) {
  // Store whether the current route is the front page?
  $is_front = \Drupal::service('path.matcher')->isFrontPage();

  // For large, `hero-style` images, set the `loading` attribute to `eager`.
  $eager_styles = [
    'hero_images',
    'responsive_carousel_hero',
    'project_hero_image',
  ];

  if (in_array($variables['responsive_image_style_id'], $eager_styles)) {
    $variables['img_element']['#attributes']['loading'] = 'eager';
    // The `hero_images` style is used for project teasers on the homepage, and
    // thus needs to be set to `lazy` only in that instance.
    if ($is_front && $variables['responsive_image_style_id'] == 'hero_images') {
      $variables['img_element']['#attributes']['loading'] = 'lazy';
    }
  }

}

This obviously isn't what I would suggest for core, but it does demonstrate how one might conditionally add "eager" to their rendered markup.

Instead, I think it might make sense for core to expose this on image field formatter as an optional configuration. Perhaps something like this?

The options would map to the supported values: auto, lazy, eager, lazy being the default. Rendered markup would respond accordingly, based on this setting. I could also see this additional display field/UI being gated behind a higher level setting if its inclusion by default is considered too heavy handed.

Tangentially related, I also wrote a simple input filter for ensuring hard-coded images in legacy content include native lazy loading attributes. Might be useful for others watching this issue.

Clarification about #29.1: it wasn't clear, but what I meant is: width and height attributes are indeed added in preprocess() if width and height variables are defined. But if the variables are not defined, there's no code to try to get them for image fields, unlike editor images where $image->getWidth() get called.
Well, I did not check: maybe it's unneeded because core always gives width and height variables for image fields. I'll give this patch a real-life test with different scenarios to ensure it works well (responsive or not, styled or not, html editor or image field).

About #30: "I could also see this additional display field/UI being gated behind a higher level setting if its inclusion by default is considered too heavy handed." Yes, I'm wondering if this UI would be necessary enough for it to be displayed by default on all websites. An opt-in might be better. In real life, is it usual that loading=lazy is inappropriate? I can't tell.
Anyway, if this patch can "break" some websites displays (can it?), we must ensure site builders can disable it.

By the way and for the record, compatibility with other image-related contrib modules like webp, svg_image and config_default_image has to be thought, but I guess it will be contrib follow-up issues.

From Google's web.dev page on implementing native lazy loading:

You should avoid setting loading=lazy for any images that are in the first visible viewport.

It is recommended to only add loading=lazy to images which are positioned below the fold, if possible. Images that are eagerly loaded can be fetched right away, while images which are loaded lazily the browser currently needs to wait until it knows where the image is positioned on the page, which relies on the IntersectionObserver to be available.

Drupal image presets and/or display formatters have no way of knowing which images will be rendered above the fold. While that same documentation page describes the impact of setting loading=lazy on images within the viewport as "minimal", it is still not zero. My vote would be, to allow those interested, to finely tune the loading value that Drupal renders.

Thank you all for your feedback!

Added test that proves loading attribute with lazy default value can be overriden.

So far we don't have support for `Responsive image` module. IMHO it could be done in a follow-up issue.

Updated the statistics and added a link to the 'loading' attribute page at caniuse.com in Issue Summary.

Just reviewed the PR again and it looks fine to me, great that this is such a straighforward change.

Added release notes snippet and change record.

Added 2 suggestions

Addressed #44. ty.

Re #38: I manually tested the latest patch in "real life" with and without image style, in image fields and in html editor. The loading=lazy attribute is correctly set, and width and height attributes are correct too! I did not test responsive images as they will be handled in #3173231: Enable lazy-load by default for responsive images.

RTBC+1 :)

The PR can't be merged and needs a rebase. I haven't actually tried out merging a PR on gitlab yet but there's always a first time.

Tagging for 9.1.0 highlights since this is a nice performance improvement that should benefit most sites.

Fixed wording and updated branch with 9.1.x changes.

Did a cli merge of the PR (was one commit behind), and pushed to 9.1.x, thanks!

So, generally, only disruptive changes are included in the release notes, and the release note should describe the disruption. (What it was like before -- what it's like now -- who is affected -- how they can fix it.) Will this issue cause any such disruptions? If so, we need to fix the release note to describe them.

"9.1.0 highlights" is what gets this mentioned on the the frontpage post as a performance improvement, so that's covered already. :)

Thanks @heddn; @catch and I agree as well.

We noticed while checking this issue and testing with responsive images that responsive images are missing width and height attribute which is bad because:

- browser can not determine the needed layout for images in advance and then when images are loaded unnecessary additional page layout and repainting will happen,
- MR in this issue is disabling lazy loading for images where width and height attributes are missing.

I've created a simple core patch in #3192234: Apply width and height attributes to allow responsive image tag use loading="lazy" which is adding width and height attribute for this case and tests are showing that this works great and eliminates both problems. Can we get somebody from this issue to maybe check the proposed patch in 3192234 and give some thoughts on it, thank you.

@pivica, responsive images are out of scope of this issue, it has been discussed in #3173231: Enable lazy-load by default for responsive images.

Any best practices for screenshot tools like puppeteer / backstopjs?
If there is an easy way to disable we might document it here. Or somewhere less obvious for me.

Thanks chrisfree for #30, how to get around forced lazy loading (Chrome is only showing ~25% of my site's images) until #3173180: Add UI for 'loading' html attribute to images is finished.

Hi, All,
I added a switch for this.
Because our sites use css to control image style. if add [height,width] to all File_Entity(image), it will affect site image layout.(attribute - height, width is the highest priority)

Hi @caohan@ciandt.com, I would suggest opening a new issue for your proposal.

Ahh, sorry, wrong issue.

This introduced a regression in EditorFileReference: #3336446-3: EditorFileReference should compute a px <img height> if a % <img width> is specified, even though % <img width> is not allowed in HTML5.