Apply width and height attributes to allow responsive image tag use loading="lazy"

Not much time right now, but in case it can help, I worked on this kind of problem in https://www.drupal.org/project/native_lazy_loading and #3173231: Enable lazy-load by default for responsive images.

I tested this and would agree that the width/height should be set by default also for responsive images. Also, the width and height should be set from the default/fallback responsive image, and not from the uploaded image itself, in order to have the correct aspect ratio.

See the screenshot attached. From the uploaded image: 1600/1000 = 1.6 aspect ratio, while the responsive image has 1339/837 = 1.533 aspect ratio.

fixed the test case.

I think this could use some explicit tests.

Nice initiative. Thanks.
I tried #10. But Responsive images were shown twice their size because I am using "Retina Images" module with x2 density parameter enabled.

Hi everyone, I've got some relevant news:

The HTML spec has very recently been updated to allow for width and height attributes to be set on source elements:

• https://github.com/whatwg/html/issues/4968
• https://twitter.com/zcorpan/status/1365046132401913861
• https://twitter.com/jensimmons/status/1365066057774493697

This means the browser will 'copy' the width and the height values to the img element for us, just like it does for the src value. Perhaps this means this work should put attributes on the source elements, rather than using the fallback width and height on the img element? Or perhaps both approaches could be used to maximise browser support?

Chromium intends to ship this feature: https://groups.google.com/a/chromium.org/g/blink-dev/c/GEjjJE0LCnM/m/0p8...

Is there a chance this will be committed before Google implements Core Web Vitals ranking signals in May?

@eeyorr, I think it's likely.

Acccording to https://chromestatus.com/feature/5737185317748736 it will be part of Chrome 90. The current stable Chrome release is Chrome 89.

https://www.chromestatus.com/features/schedule shows Chrome 90 as reaching a stable release around mid-April.

Edit: Sorry, I've just realised eeyorr probably means 'Will this Drupal issue be closed in time for May' whereas I thought they were asking about the change to the HTML spec that I had mentioned in an earlier comment.

Here's where I've got to on this:

1. Agreed that using the fallback image style to set the H/W on the img tag works best.
2. @pivica also agree it is then necessary to set width:100% and height:auto on each image in the css to maintain expected aspect ratios.
3. @Phil Wolstenholme agreed that we should set H/W attributes on the source tag as well as img to fully support Art Direction and allow the browser to choose fully responsive images with (potentially) different aspect ratios.
4. … although as @Phil Wolstenholme says, putting H/W in the source tag still isn’t working in Chrome 89 or Firefox 87 and we’ll have to wait for the next release to test it. I’ve created a simple codepen where you can still see janking in action: https://codepen.io/danielward99/pen/RwKjEP
5. In preparation for Chrome 90, should we have another look at the way the module outputs picture elements…

At the moment, the responsive image module outputs each image style into a single comma-separated srcset attribute list:

<picture>
<source srcset="/sites/default/files/styles/max_325x325/public/2021-04/large.jpeg?itok=OBosLXN1 325w, /sites/default/files/styles/max_650x650/public/2021-04/large.jpeg?itok=WS4TBSbZ 650w, /sites/default/files/styles/max_1300x1300/public/2021-04/large.jpeg?itok=jt9SfiL8 1300w, /sites/default/files/styles/max_2600x2600/public/2021-04/large.jpeg?itok=0GCvKdCg 2350w" type="image/jpeg" sizes="(min-width: 1290px) 1290px, 100vw">
<img srcset="/sites/default/files/styles/max_325x325/public/2021-04/large.jpeg?itok=OBosLXN1 325w, /sites/default/files/styles/max_650x650/public/2021-04/large.jpeg?itok=WS4TBSbZ 650w, /sites/default/files/styles/max_1300x1300/public/2021-04/large.jpeg?itok=jt9SfiL8 1300w, /sites/default/files/styles/max_2600x2600/public/2021-04/large.jpeg?itok=0GCvKdCg 2350w" sizes="(min-width: 1290px) 1290px, 100vw" src="/sites/default/files/styles/wide/public/2021-04/large.jpeg?itok=2Bj04ZKZ" alt="A forest with some trees in it." typeof="foaf:Image">
</picture>

But to make it possible to set H/W on each image style in this responsive group, how about outputting a separate source tag for each breakpoint:

<picture>
        <source
            media="(max-width: 799px)"
            srcset="https://picsum.photos/1000/1500"
            type="image/jpeg"
            width="1000"
            height="1500"
        >
        <source 
            media="(min-width: 800px)"
            srcset="https://picsum.photos/2350/1500"
            type="image/jpeg"
            width="2350"
            height="1500"
        >
         <img srcset="/sites/default/files/styles/max_325x325/public/2021-04/large.jpeg?itok=OBosLXN1 325w, /sites/default/files/styles/max_650x650/public/2021-04/large.jpeg?itok=WS4TBSbZ 650w, /sites/default/files/styles/max_1300x1300/public/2021-04/large.jpeg?itok=jt9SfiL8 1300w, /sites/default/files/styles/max_2600x2600/public/2021-04/large.jpeg?itok=0GCvKdCg 2350w" sizes="(min-width: 1290px) 1290px, 100vw" src="/sites/default/files/styles/wide/public/2021-04/large.jpeg?itok=2Bj04ZKZ" alt="A forest with some trees in it." typeof="foaf:Image" width="1000" height="1500">
</picture>

I’ll have a go at a patch if this is the right approach.

Hi @bingolitte,

I think there's already an option for Drupal to use multiple source elements when necessary, for example if different type values are provided.

You can see an example of that on https://api.drupal.org/api/drupal/core%21modules%21responsive_image%21re... :

<picture>
  <source [...] mime-type="image/webp" srcset="image1.webp 1x, image2.webp 2x, image3.webp 3x"/>
  <source [...] mime-type="image/jpeg" srcset="image1.jpeg 1x, image2.jpeg 2x, image3.jpeg 3x"/>
  <img src="fallback.jpeg" />
</picture>

I think the srcset vs source elements with a media attribute depends on how the responsive image style is configured in Drupal, particularly around breakpoints.

The blog post linked to from this comment might be useful in explaining the configuration differences, although I admit I don't fully understand them as I always end up with a picture element with separate source elements as I use responsive images to provide WebP versions of each source.

Hi @Phil Wolstenholme,

#20 is where I started messing about with this. I think we're talking about the same blog post from ChromaticHQ. I originally followed that to get Drupal to output a picture element instead of an img tag. But for me, it defaults to a single source with multiple image styles in the srcset, rather than separate ones. Same as you, I separate my source elements out programatically in order to provide WebP support, but I don't know if there's a way to do that in the configuration.

#19 is super interesting. I need to go away and work through the helper function in more detail. It looks like it can output at least 11 combinations of picture, source and srcset, although how they all relate to the Drupal configuration...

Yes, you can get different aspect ratios for different viewports (art direction) through the use of different source tags.
A real life example :

<picture>
    <source srcset="/system/files/styles/3_1_lowest/private/images/senior-couple-afternoon-tean-drinking-relax-concept-low.jpg?itok=N4-p1OFm 100w, /system/files/styles/3_1_verylow/private/images/senior-couple-afternoon-tean-drinking-relax-concept-low.jpg?itok=R_U6uG_H 200w, /system/files/styles/3_1_low/private/images/senior-couple-afternoon-tean-drinking-relax-concept-low.jpg?itok=AgEz9BPH 400w, /system/files/styles/3_1_medium/private/images/senior-couple-afternoon-tean-drinking-relax-concept-low.jpg?itok=XpzI5AVQ 800w, /system/files/styles/3_1_high/private/images/senior-couple-afternoon-tean-drinking-relax-concept-low.jpg?itok=efrF7yRT 1280w, /system/files/styles/3_1_veryhigh/private/images/senior-couple-afternoon-tean-drinking-relax-concept-low.jpg?itok=2P2rnKGR 1440w, /system/files/styles/3_1_highest/private/images/senior-couple-afternoon-tean-drinking-relax-concept-low.jpg?itok=Kz4L9ulL 2048w"
            media="(min-width: 768px)" type="image/jpeg" sizes="100vw">
    <source srcset="/system/files/styles/1_1_lowest/private/images/senior-couple-afternoon-tean-drinking-relax-concept-low.jpg?itok=1njkdq9T 100w, /system/files/styles/1_1_verylow/private/images/senior-couple-afternoon-tean-drinking-relax-concept-low.jpg?itok=RLlyttuh 200w, /system/files/styles/1_1_low/private/images/senior-couple-afternoon-tean-drinking-relax-concept-low.jpg?itok=JaqoqRli 400w, /system/files/styles/1_1_medium/private/images/senior-couple-afternoon-tean-drinking-relax-concept-low.jpg?itok=P8u780hB 800w, /system/files/styles/1_1_high/private/images/senior-couple-afternoon-tean-drinking-relax-concept-low.jpg?itok=_SBv7u8M 1280w, /system/files/styles/1_1_veryhigh/private/images/senior-couple-afternoon-tean-drinking-relax-concept-low.jpg?itok=OSknyKTT 1440w, /system/files/styles/1_1_highest/private/images/senior-couple-afternoon-tean-drinking-relax-concept-low.jpg?itok=QU9M-sms 2048w"
            type="image/jpeg" sizes="100vw">
    <img
            src="/system/files/styles/3_1_highest/private/images/senior-couple-afternoon-tean-drinking-relax-concept-low.jpg?itok=Kz4L9ulL"
            alt="Les vertus du café" title="Les vertus du café" loading="lazy" typeof="foaf:Image" width="2048"
            height="683">
</picture>

To do that with Drupal, you must first define multiple breakpoints, usually in YOURTHEME.breakpoints.yml (https://www.drupal.org/docs/theming-drupal/working-with-breakpoints-in-d...).
Then, in the responsive image style configuration, you choose your breakpoints list in the "Breakpoint group" field. When doing so, the form is updated so that you get to configure your responsive image style for each breakpoint, i.e. each source tag.

NB: to duplicate source tags for webp support, you can use the webp contrib module.

I just want to let people interested know that patch #10 works for me.
See attached images.

I have a project using responsive image styles with breakpoints (based on bootstrap) defined in my theme.
I was running lighthouse test in chrome on the project, noticed that it reported "Image elements do not have explicit width and height".
I applied patch #10 et voila, one less issue on the lighthouse report.

p.s. image nr 3 is named badly, it is of course lighthouse diagnostic AFTER applying the patch.
p.s.2: also works on Drupal 9.1.5 btw

Patch #10 works for me to add H/W attributes to the fallback img. I've now added Patch #25 to set the H/W attributes on each source as well. It takes into account the three options in the responsive image configuration:

Where the Select a single image style. option is selected for a given breakpoint, the ouput is straightforward: only one image served up so I have used the H/W of that single styled image to determine the attributes.

Where the Select multiple image styles and use the sizes attribute. option is selected, I have used the last styled image in the breakpoint (ie: the largest image) to determine the H/W. This relies on each image in the breakpoint using the same aspect ratio, but it seems unlikely that someone would want to use different aspect ratios at the same breakpoint. In any event, providing the browser with at least one of the aspect ratios correctly is maybe than none.

Where the Do not use this breakpoint. option is selected, no source is served up at this breakpoint and the fallback img is used instead with the H/W already set by Patch #10.

@bingolitte this patch works well for a Drupal 8 project we are working on. This has always been a troublesome area especially for Lighthouse scoring. FYI we are using the img srcset only as in most cases that is the preferred vs the picture wrapper. I noticed most of the comments are related to picture and just wanted to let you know it works on img srcset as well! +1 RTBC.

I just gave the patch in #25 a test run on a 8.9 and 9.1 site where we use a lot of responsive images that have separatesource elements for each type (jpeg & webp) and breakpoint - it all seemed to work well.

I had to edit my image template to get the fallback img width and height to appear as attributes, but I think that was just an issue with my theme.

I think we'll still need some more tests (see #11) to get this into core, but good work so far @bingolitte, @pivica and @nikitagupta!

Patch #25 is working fine for me on Drupal 8.9.15

Google search console was complaining that the Cumulative Layout Shift ( https://web.dev/cls/ ) was high on my site. This patch will help with it. Thanks!

Patch #25 is working fine for me on Drupal 8.9.15

Hi all. Here's where I've got to with writing some tests for this patch:

The two key-value-pairs added by @nikitagupta in #10 to doTestResponsiveImageFieldFormatters successfully test the addition of H/W attributes to the fallback image.

I've now written two new assertions in doTestResponsiveImageFieldFormatters to test the addition of H/W attributes to the source image (patch #30).

I've run the tests as assertRaw and assertNoRaw both with an without patch #25 applied and the pass/fail results are as expected.

@Phil Wolstenholme do you think that covers with the testing requirement, or do you think it needs something else?

I think if all the new functionality that has been added has a corresponding test then that's probably great, but I'm not a core maintainer or anything like that :)

Patch 3192234-30.patch in #30. fixes the issue for me.
Height and width are visible now. Adding before and after patch screenshot for reference.

Patch included test and fixes the issue. Moving to RTBC.

#30 applies cleanly, but still having issues when a page has images with no image styles, like when an image field display is set to Rendered Entity.
The issue appears here :
ImageStyle::load($responsive_image_style->getFallbackImageStyle());
So it gets null.
I don't know if there is a better solution cuz I haven't been following the issue, but I worked around it for now by checking the $image_style if null or not, and do nothing if it is.

Updated the patch in #30 with a condition suggested by #34, thanks!

The patch at #35 seems to be working as expected for me (9.1.x).

Possible bikeshedding, but I think it hasn't been sufficiently discussed:

@bingolitte wrote:

Where the Select multiple image styles and use the sizes attribute. option is selected, I have used the last styled image in the breakpoint (ie: the largest image) to determine the H/W. This relies on each image in the breakpoint using the same aspect ratio, but it seems unlikely that someone would want to use different aspect ratios at the same breakpoint.

We have some very large sizes in our srcset attribute, to allow for large retina displays, etc. I think I would prefer the image width and height attributes to reflect the size of the fallback image, ie the style referenced in the image src attribute. That gives you an img element that would size as expected on absolutely any browser.

What is the reasoning around using the largest possible size here? If it's unlikely anyone would serve up different aspect ratios in the same srcset (though I can see use-cases for doing so), then it's also unlikely they would use a different aspect ratio for the fallback style, right?

Hi @John Pitcairn,

Assuming the images are responsive with img { width: 100%; height: auto; } set in the CSS, the images should take the width of the parent container, irrespective of the dimensions in the H/W attributes. The browser just uses H/W to calculate the aspect ratio of the expected image and reserve the correct ratio of space in the layout accordingly. In theory, we could use any reasonable numbers for the H/W attributes, so long as they return the correct aspect ratio. There's an in-depth explanation in this blog post.

In this method, I've chosen to use the largest image style only out of expedience. As long as they have the same aspect ratio, we could use any of the image styles in this breakpoint to determine the attributes. But the largest style is the last item in the responsive_image_styles array, which means we only need minimal additional code to achieve the result.

That said, if you're getting behaviours different from these, are you able to share some code snippets and screenshots?

I understand that. I was really just thinking of providing the width and height of the actual fallback src image, so the tag becomes a very standard img tag for any browsers that don't understand the srcset attribute. It seems potentially more backwards-compatible, though I have no good example so maybe it really doesn't matter. Don't consider this a blocker.

Hi @John Pitcairn understood now. The H\W attributes were added to the fallback image in Patch #10, but if you're seeing something different in the output, upload the screenshot and I'll take a look.

@bingolitte: I think there may be an unforeseen consequence of using the largest responsive image size. Google's lighthouse tool is now reporting a lower performance score for the site, because it thinks there are huge images that should be resized to reduce bandwidth. I will try to confirm this in a with/without patch scenario.

Another reason to use the fallback image's dimensions rather than the largest image's dimensions might be that if the CSS fails to load or is being lazyloaded and has not been loaded yet then the browser will use the width and height attributes to set the image's dimensions on the page.

In these situations, a smaller image might be less disruptive to the page than a giant one. A giant one could cause horizontal scrolling or obscure content by pushing it down quite far if we were showing a 2x resolution desktop monitor sized image to a user on a phone that went through a train tunnel just as the CSS file for the page was trying to load.

It seems even though the patch is set to use the Fallback:

$image_style = ImageStyle::load($responsive_image_style->getFallbackImageStyle());

It is not. We are getting the largest height and width set.

Now this may be due to the way in which we are using responsive image styles.

We use 1x Viewport Sizing
Select multiple image styles and use the sizes attribute.
and select all the styles

This way we are using img srcset as apposed to the picture element.

@glynster: We are doing the same.

@John Pitcairn ok good to we can replicate it. Just seems odd that the comment says:

+ // Get width and height from fallback responsive image style and transfer them
+ // to img tag so browser can do aspect ratio calculation and prevent
+ // recalculation of layout on image load.

Debugging to see what might be the cause.

Updated patch #35 to make sure the fallback is being set. @John Pitcairn can you confirm this works for you? Works on my end, was a very small tweak.

Confirmed, the patch at #47 now provides width and height attributes from the fallback image when using the srcset attribute for an <img> element. Thanks @glynster.

1. +++ b/core/modules/responsive_image/responsive_image.module
   @@ -212,6 +212,26 @@ function template_preprocess_responsive_image(&$variables) {
   +  // We are assuming that all image styles in this responsive image have the
   +  // same aspect ratio.
   

   This feels like a pretty big assumption

   Isn't the whole point of responsive design and the responsive image supposed to be that you serve the best image resolution for the breakpoint? What are the implications here?

2. +++ b/core/modules/responsive_image/responsive_image.module
   @@ -212,6 +212,26 @@ function template_preprocess_responsive_image(&$variables) {
   +    if (!file_exists($build_uri)) {
   +      $image_style->createDerivative($variables['uri'], $build_uri);
   

   We can't go doing this in a preprocess function. See \Drupal\image\Controller\ImageStyleDownloadController::deliver we have a whole lock system wrapped around image-style generation to prevent race conditions between multiple threads.

   Doing it here means we're bypassing all of that stampede protection. I think we're going to need to think about what happens here when the image doesn't exist and the server is under concurrent load.

+++ b/core/modules/responsive_image/responsive_image.module
@@ -212,6 +212,26 @@ function template_preprocess_responsive_image(&$variables) {
+    $build_uri = $image_style->buildUri($variables['uri']);

Agreed with @larowlan about the file i/o here - we have image styles specifically to avoid doing anything in actual page requests.

What about using ImageStyle::transformDimensions() here? That should have the same effect without any major overhead.

I also have the same question about what happens if the height and width are wrong - will it be corrected (requires a recalculation, but we already have one) when the image is actually loaded or are we risking displaying the image in the wrong aspect ratio?

@larowlan @catch you are both right here. The patch is assuming and hooking in after the fact. My knowledge is limited and I just worked off the proposed patch. For me the patch has been useful for lighthouse scoring and best practices for images to have a width and height set.

I guess this means there is no way to sensibly set those attributes in the html source for an image srcset, unless you make the assumption that the aspect ratio does not change.

There are definitely sites I've built where the aspect ratio of images does change at different breakpoints - though for that I wouldn't use srcset.

I wrote some custom code in a theme a few weeks ago that loops through all the widths and heights and checks that they are the same aspect ratio (or close enough, to account for human errors when configuring each image style width and height). Maybe we could use something like that to check the aspect ratios and ensure they're the same for each breakpoint?

Another thing to discuss is if it matters too much if we do accidentally set incorrect width and height attribute values in some cases? My understanding is that once the image has loaded then incredibly common CSS rules like img { width: 100%; height: auto; } would mean that the image attributes would get ignored anyway. The attributes are most useful for preventing layout shift issues as an unloaded image takes up no space. If we provided incorrect width and height attributes then there would still be some layout shift, but not as much as if we provided no width or height attributes at all. But as long as that CSS snippet is on the site then I don't think we'd end up with distorted images, as the CSS would override the attribute values.

Maybe responsive image module should supply that css by default?

I got my CSS snippet a bit wrong earlier, it's this one that many people use:

img {
  max-width: 100%;
  height: auto;
}

I just checked and Bartik, Claro, Seven, and the Umami theme all include it. However the max-width vs width distinction is important and might mean that some of the info in my last comment about the CSS overriding the width attribute is wrong unfortunately, sorry about that everyone.

The patch with transformDimensions() instead of createDerivative().
Dimensions are output for <source> tags if possible.

Just tested the patch from #56 on a D8 site I'm trying to get better PageSpeed scores on. Patch applied cleanly on 8.9.16, width and height are output as expected, and Google has stopped complaining about the missing width and height.

Worth noting this site only has one image style defined for use with WebP so all srcset images and the fallback have the same dimensions.

Patch 56 applied nice but for responsive images Firefox and Safari read them wrong.

Firefox and Safari are using width and height form < img part of picture instead of < source
Chrome and resend Edge do it right and use < source width and height.

Any solution for this?

Reroll for 9.3.x. This won't apply on 9.2 anymore.

Is there any concern that adding loading="lazy" to an image that happens to be the LCP element will cause such an image to be prioritized lower than usual? I'm not quite sure how to definitively benchmark this, but it seems at least in Chrome 81, that adding the loading attribute causes an LCP image to be loaded at position #25 as opposed to position #15 on an arbitrary Drupal 9 site that I just tested it out on. A pile of non-lazy SVG images seem to be prioritized and downloaded first...which is not what we would want for LCP performance.

Theoretically, LCP images should be eagerly loaded in order to cause the FCP timing to happen as early as possible.

I'm going to set this back to "Needs Work" until someone can either confirm or dismiss my concerns. If this is indeed the case, then I think that we'll have to make a toggle for this on a per-image-style basis.

Thanks.

@Luke, I don't think this patch does anything relating to the `loading` attribute, it just adds `width` and `height` attributes.

The lazy loading attribute comes from https://www.drupal.org/project/drupal/issues/3167034. You're right about lazyloading not always being the right choice and having an impact on LCP for hero images, which is why there is an issue (https://www.drupal.org/project/drupal/issues/3173180) to add a UI to be able to turn it on and off.

@Phil - I can understand that the patch doesn't directly add 'lazy', but indirectly it does cause the attribute to be applied to images generated by responsive image formatters.

Functionally, this patch, as it stands, could have a detrimental impact on quite a few sites unless I'm missing something obvious?

At any rate, I think that blocking this task until the UI lands is a smart move. A warning in the I.S. might also be useful to prevent people from applying this patch without taking the proper benchmarking steps, which in my experience is a lot more effort than most folks are willing to go through :). Our LCP went up by ~400ms when applying this patch.

On a good note, it is very easy to rectify the performance hit, should any adventurous shops want to give it a try.

/**
 * Implements template_preprocess_responsive_image().
 */
function MY_THEME_preprocess_responsive_image(&$variables) {

  $eager_styles = [
    'hero_banner_style_1',
    'hero_banner_style_2',
  ];

  if (in_array($variables['responsive_image_style_id'], $eager_styles, TRUE)) {
    $variables['img_element']['#attributes']['loading'] = 'eager';
  }

}

@Luke, ah yes, of course - Core won't add the native lazy loading unless the image has a width and height set, and this patch adds a width and height for responsive images, so now they'll be made to load lazily, I get it now.

Personally, I'm not sure the performance issue should block this issue though, it didn't block https://www.drupal.org/project/drupal/issues/3167034 which did the same thing for non-responsive images?

https://www.drupal.org/project/drupal/issues/3173180 has some discussion around adding the UI to the responsive image formatter, which is good.

The more that I think about it, I keep coming back to the fact that #3167034 may have introduced a performance regression and that the UI work should be promoted to a major issue.

Given that #3167034 was closed out 9 months ago, it is reasonable to conclude that core web vitals wasn't on anyone's radar at the time.

Performance really is a tricky beast to tackle.

I've just seen Rar9's comment:

Patch 56 applied nice but for responsive images Firefox and Safari read them wrong.

Firefox and Safari are using width and height form < img part of picture instead of < source
Chrome and resend Edge do it right and use < source width and height.

Any solution for this?

At the moment using the width and height attributes from the source elements is a Chromium only feature.

There is a Firefox bug here: https://bugzilla.mozilla.org/show_bug.cgi?id=1694741
And a Safari bug here: https://bugs.webkit.org/show_bug.cgi?id=222368

Fingers crossed all browsers support this eventually!

Given that we have #3173180: Add UI for 'loading' html attribute to images to eventually toggle the lazyloading behaviour and a workaround in comment #63, I don't think this issue (which is mostly concerned with fixing CLS issues) should be in 'needs work' because of its interaction with the lazyloading behaviour. That said, I am happy to be disagreed with :)

I agree that it is really important to be able to turn off lazyloading for big images to prevent LCP issues, but I think the place to help push for that change is in #3173180: Add UI for 'loading' html attribute to images, and this issue should focus on the CLS (holding space for images by setting width and height attributes) issue.

I think I've found an issue with #59 when using the 'Select a single image style for each breakpoint approach' and when using a 1x and 2x multiplier.

On line 42 of the patch we check to see if count($srcset) === 1:

+  if (count($srcset) === 1 && !empty($dimensions['width']) && !empty($dimensions['height'])) {
+    $source_attributes->setAttribute('width', $dimensions['width']);
+    $source_attributes->setAttribute('height', $dimensions['height']);
+  }

In my case this check failed as count($srcset) was equal to 2 – one entry for the 1x multiplier and one entry for the 2x multiplier:

I'm not sure what the best fix is here. What was the purpose of the count($srcset) === 1 check? Could we replace it with count($srcset) > 0? Or maybe (count($srcset) === 1 | $image_style_mapping['image_mapping_type'] == 'image_style')?

I'm not sure this is the best approach, but here is a patch that swaps count($srcset) === 1 for (count($srcset) === 1 | $image_style_mapping['image_mapping_type'] == 'image_style').

I have tested this with a responsive image style using the 'Select a single image style for each breakpoint approach' and using a 1x and 2x multiplier, but I've not tested it with the 'sizes' approach.

Edit: I've noticed my changes mean that the dimensions are coming from the 2x multiplier so they are double what they would be for the 1x modifier, but they still do the job of setting a correct aspect ratio. There's probably something nicer we can do here to avoid this.

Sorry for the lack of an interdiff, patchutils gave me a interdiff: Whitespace damage detected in input message and produced an empty interdiff when I tried to use it.

+++ b/core/modules/responsive_image/responsive_image.module
@@ -421,6 +432,10 @@ function _responsive_image_build_source_attributes(array $variables, BreakpointI
+  if ((count($srcset) === 1 | $image_style_mapping['image_mapping_type'] == 'image_style') && !empty($dimensions['width']) && !empty($dimensions['height'])) {

I think | should be || here.

Thanks @longwave.

I was in a bit of a rush - here's a revised patch that uses ||. I think I'll step away from this issue for a bit and work on something else to avoid making any more silly mistakes :)

…and here's another revised version where the patch doesn't contain a previous patch that I accidentally included in the git diff that I used to make the patch! 🤦‍♂️

Reroll patch Core 9.2.5

Due to some missing changes in #74, tests is fail. Added a patch with all the changes, Please have a look.

#76 wont apply for 9.2.x , so for now will continue with #56

I edited patch #76 to fix the HTML validation errors:
Error: Attribute height is not allowed on the element source at this point.
Error: Attribute width not allowed on element source at this point.
so no need for width and height on the source tag.

Thanks #81 works nicely on d9.2.6

Unfortunately, patches #79 - #81 contradict the very soul of this task: to add attributes which help browsers to reserve proper space for not yet loaded images. Modern browsers start to support these attributes for source element.

Straight re-roll for #81 on 9.3.x branch.

Here is a reroll of #84, with the source attributes added back in per the comment in #83 for 9.3.x.

#83: It would be helpful to get links to various browser vendors' intentions on this subject before adding width and height attributes to <source> elements so that we know exactly how they're going to interpret them.

Verified and tested patch #85.
Patch applied successfully and looks good to me.

Testing Steps:
# Goto: Appearance -> Apply Seven theme
# Goto: Extend -> Install "Responsive Image" module
# Goto: admin/structure/types/manage/page/fields
# Add Image field
# Goto: admin/structure/types/manage/page/display
# Select "Responsive Image" under format
# Upade it
# Create respective content type with image where user added the image field
# Save it
# Inspect element and goto on that uploaded image
# Check the results.

Expected Results:
# After applying patch #85, the User should see Width and Height attributes appearing for the responsive image tags.

Actual Results:
# Width and Height attributes are not appearing for the responsive image tags.

Please refer attached screenshots for the same.
Looks good to me.
Can be a move to RTBC.

Re #83/86 width and height attributes on the source element is part of the HTML5 spec:

Relevant quotes from https://html.spec.whatwg.org/multipage/embedded-content.html#the-source-...

The source element's parent is a picture element
...
The source element supports dimension attributes. The img element can use the width and height attributes of a source element, instead of those on the img element itself, to determine its rendered dimensions and aspect-ratio.
...
The width and height attributes on an img element's dimension attribute source map to the dimension properties 'width' and 'height' on the img element respectively. They similarly map to the aspect-ratio property (using dimension rules) of the img element.

To me this implies that conforming browsers should use the width/height found on source to replace the width/height on img when rendering responsive images.

The patch at #85 only works if your responsive image sources all have the same aspect ratio, which matches the fallback img ratio.

This comment is still true: https://www.drupal.org/project/drupal/issues/3192234#comment-14220803 -- the last patch that accomplished the goal of this ticket was in #76.

#88 That's what I was looking for, thanks!

Updating IS. I think we need a decision on where we want to put the dimensions.

I've removed the responsive image implementation of lazy loading from #3173180: Add UI for 'loading' html attribute to images so as to move that into here. This way we can add dimensions at the same time we add lazy loading support. Or if that gets too complicated in _this_ issue, we can land dimensions first and lazy loading with a UI toggle as a follow-up. Although I think the lesson learned from hard coding loading="lazy" first time around for <img> was that it caused negatively affects to the page's LCP score. So I think I would advocate for doing enabling lazy loading and a UI to disable it (loading: eager) all at the same time.

What patch to use with Drupal 9.27?
#81, #84 or #85 are no longer working

Re-titling.

OK, this (hopefully) makes things a little more happy. This adds dimensions to the <source> and <img> tags. For browsers that support it, they should leverage the sizes on the <source> tag in favor over <img>. This also adds the field formatter changes from #3173180: Add UI for 'loading' html attribute to images that are appropriate to responsive image. And adds update tests for both field formatter and views fields.

This could use some reviews at this point. I still want to beef up test coverage of dimensions added to the <source> tags, but there is enough working code here that a review would beneficial.

Sorry for such a lot of changes. If you are at all familiar w/ the latest patches in #3173180: Add UI for 'loading' html attribute to images, then the review should be pretty easy here as there's wide copy/paste between these 2 issues. Perhaps that means we should create a trait? I'm not sure.

#98 Still wont apply to 9.27 :-(

@Rar9: I'm seeing this error too with the previously working patch from #56 (haven't tried newer ones). I ran composer update drupal/core* --verbose and it looks like it's failing on ResponsiveImageFieldDisplayTest.php, but the rest patches okay:

    https://www.drupal.org/files/issues/2021-06-23/3192234-56.patch (Apply width and height attributes to responsive image tag (core 9.2.x) #3192234: Apply width and height attributes to allow responsive image tag use loading="lazy": https://www.drupal.org/project/drupal/issues/3192234 )
patch "-p2" --no-backup-if-mismatch -d "drupal/core" < "C:\Users\i\AppData\Local\Temp/615f017ceae36.patch"
patching file modules/responsive_image/responsive_image.module
patching file modules/responsive_image/tests/src/Functional/ResponsiveImageFieldDisplayTest.php
Hunk #1 FAILED at 309.
1 out of 2 hunks FAILED -- saving rejects to file modules/responsive_image/tests/src/Functional/ResponsiveImageFieldDisplayTest.php.rej

@Rar9 this issue is for 9.3.x if you need this for 9.2.x you'd have to backport the latest patch.

re: applying on 9.2. There are some test only changes in 9.3 that makes the patch here fail to apply on 9.2. I recommend you create a patch without tests and include that local patchfile in your development workflow instead.

Here's some test coverage and a small fix to the formatter. Removing the needs tests tag. I marked this earlier as a task, without a comment. But that is really what we have here. Work that needs to be done. There's no bug per-se.

@Rar9 Here's #56 with the test removed as per @heddn's suggestion. It applied without errors on 9.2.7 for me.

Re-roll patch to drupal 9.2.7

The last uploaded patch needs to be for 9.3. If we upload another patch for another branch, please also include the 9.3 patch too. Otherwise the testbot can't run tests and keep the most recent patch front and center for reviewing.

Re-uploading #103.

Updated comment and implementation on when to add dimensions to the srcset. Feedback requested.

Some more docs updates.

Resolved custom commands failure & also added an interdiff for review.

.

Is this patch suppose to add a UI for setting width/height when embedding a media?

No.

RTBC - After extensive review this patch looks great to me.

1. +++ b/core/modules/responsive_image/config/schema/responsive_image.schema.yml
   @@ -67,3 +67,10 @@ field.formatter.settings.responsive_image:
   +    image_loading:
   +      type: mapping
   +      label: 'Image loading settings'
   +      mapping:
   +        attribute:
   +          type: string
   +          label: 'Loading attribute'
   

   Why do we need a parent and child key here? Do we expect to add more items under "image_loading"?

2. +++ b/core/modules/responsive_image/src/Plugin/Field/FieldFormatter/ResponsiveImageFormatter.php
   @@ -186,7 +212,12 @@ public function settingsSummary() {
   +    $summary[] = $this->t('Loading attribute: @attribute', [
   

   The test failure shows that we weren't testing this summary. Should we also test the summary when it's set to eager?

3. +++ b/core/modules/responsive_image/tests/src/Functional/ResponsiveImageFieldDisplayTest.php
   @@ -318,6 +318,10 @@ protected function doTestResponsiveImageFieldFormatters($scheme, $empty_styles =
   +      // Assert the output of the 'width' attribute.
   +      $this->assertSession()->responseContains('width="360"');
   +      // Assert the output of the 'height' attribute.
   +      $this->assertSession()->responseContains('height="240"');
   

   Should we test the loading attribute here?

4. +++ b/core/modules/views/src/ViewsConfigUpdater.php
   @@ -480,6 +480,50 @@ protected function mapOperatorFromSingleToMultiple($single_operator) {
   +  protected function processReponsiveImageLazyLoadFieldHandler(array &$handler, string $handler_type, ViewEntityInterface $view) {
   

   Reponsive -> Responsive (should cspell have caught this?)

115.1: I don't think there will be more later. But to handle testing the upgrade path in ResponsiveImageLazyLoadUpdateTest and the way that entity view displays inject their default settings, it gets a lot more complicated to test this thing. Is there a strong reason _not_ have the nesting?
115.2: We have a functional test of both eager and lazy already.
115.3: Done
115.4: Fixed

Back to RTBC

Just a re-roll of #56 to apply to the latest Drupal 9.2.x (seems folks are looking for a patch that'll apply to 9.2.x, but there isn't one available in this issue anymore).

Hope to see this issue committed soon :-).

Installed and tested patch #116 under Drupal 9.3.0 and seems to work correctly. Thanks for the effort.

@MaxMendez we have also tested patch #116 using Drupal 9.3.0 and works correctly. RTBC +1.

Implemented #119 today and the patch applies, but I am not finding any height and width attributes on the <source> elements. Digging through the comments, I suspect that some of the points made in #123 are valid for the patch for 9.2.x, and the reason that the attributes are not present.

@pivica, great find! In your research of the problem, did you find if this was a browser specific issue? From reading https://www.smashingmagazine.com/2020/03/setting-height-width-images-imp..., I get the distinct impression that the fallback image and its size should only be used in a fallback scenario.

I hope it is browser or fallback specific. Because if it isn't, we might need to go back to the drawing board a bit. We don't really want to set a fallback image that is large. That would mean all the poor mobile users with old cell phones on slow data networks will have to download large images. But we also don't want to set the dimensions on that fallback image to the large size with a tiny image. That would lead to bad experience for those same poor users.

I'm asking a few folks to see if they have an opinion on the best thing to do here - or whether some other CMS implements a best practice that could be adopted here. Unfortunately a lot of people are still on vacation, so I might not get a great answer until next week...

One thing I wanted to clarify, as I'm not familiar with image size options in Drupal... only browsers that don't support srcset would use this fallback image. (I think that's just IE and Opera Mini.) From what you're saying here, the various sizes specified in srcset wouldn't necessarily share the same dimension ratio... is that right? In that case, is there ever a fallback image that you could be sure would be the right dimensions, and thus expand to the proper size?

In other words, would the fallback image often be in the wrong aspect ratio anyway?

#129
worked,
thank you very much

@pivica perhaps I misunderstood the issue you're describing in #128. I thought you were pointing out that, when the browser expanded the small fallback image, blank space was left because it had a different height/width ratio than the container. I think now that you were not saying that all :)

You were just saying that the small image looked lousy when expanded to a larger container. Which can certainly be the case. If dimensions are consistent among the generated images, then this problem is solved if the fallback image is the largest size, not the smallest. Except that users who are using IE (or especially Opera Mini), which don't support srcset, might well also be more likely to use slower connections, on which the larger image can be an unnecessary hardship if it's being displayed in a smaller container after all.

Do I understand the issue correctly?

@pivica, I think where Ben and I are getting stumped up is

Following this, should we change 'Fallback image style' field description on responsive image style admin UI? Currently it is saying:

Select the smallest image style you expect to appear in this space. The fallback image style should only appear on the site if an error occurs.

And this is not correct any more now when we are injecting width and height attributes because if you select smallest image style which is smaller then your parent wrapper where you are putting your image and if you have above regular CSS rule for image then image will be smaller then you would actually want it to be. Description should recommend something like - select image style which is bigger or same size as your maximum width of parent wrapper of an image on the page.

If we are supposed to change to a larger image, doesn't this impact the very users we want to support? The users that can't handle <img srcset=""/>? Although from looking at https://caniuse.com/srcset, the sites that can't handle srcset are very few.

Hmm, what if we don't set the image width/height on the <img/> tag? That would result in layout shift, but would it only be for the fallback image? Meaning, not for the 95% of browser users that support <img srcset=""/>.

Re #136: I don't think that will work. We are better off updating the wording of the fallback image. Or, just "do the right thing". I think the "right thing" in this case is to use the largest image size (first image) for the height/width on the <img> tag. In those cases where we aren't using <picture>

Just noting, as per conversation just now with @heddn, that Opera Mini is widely used in Africa on mobile devices.

According to https://gs.statcounter.com/browser-market-share/mobile/africa , it's got a 12% market share on mobile devices. Lots of these users will be facing 2G or sub-2G bandwidth and/or data restrictions. So it would be nice to serve them smaller images by default.

On the other hand, IE users are of course likely to be on desktop devices, where large images would be fine :)

I might also guess that Drupal sites are somewhat more likely to be used on desktop devices than the average site... so that would tilt the scales back in favor of serving the largest image. Which, from the discussion here, sounds like what needs to be done anyway.

Whether the fallback image should be large, small, the same aspect ratio or otherwise is a decision for the designer and site-builder, based on known or assumed metrics about the site's intended user base, the theme breakpoints in use, and how those apply to the layout. A larger screen does not always equal larger images. This should not be baked into core (or core help text) as an assumption.

Conflating the fallback image dimensions with the image element width and height attributes is possibly not optimal as a result. A better mechanism could be proposed, but once again it should be a decision for the designer and site-builder. This might be to optionally allow separately selecting a style to provide the size attributes, or providing two integer fields to allow an optional manual override, etc.

Might this be best addressed as a follow-up issue?

NW to implement #137/#138.

Some possible language for the fallback description:

Select the image style you wish to use as the style when a browser does not support responsive images.

According to https://gs.statcounter.com/browser-market-share/mobile/africa , it's got a 12% market share on mobile devices. Lots of these users will be facing 2G or sub-2G bandwidth and/or data restrictions. So it would be nice to serve them smaller images by default.

On the other hand, IE users are of course likely to be on desktop devices, where large images would be fine :)

We're officially dropping IE support in Drupal 10, but not support for Opera mini, so that puts a bit more weight on using the lowest bandwidth option for the fallback. I also don't think it hurts to default to the lower bandwidth option in Drupal 9 where we support both - IE users have a lot of options compared to a phone on 2g/3g and at worst they get a little image instead of a page that never finishes loading.

Also note that we did drop (nominal) support for one segment of Opera Mini recently, that had never been supported but was incorrectly specified: #3202818: Remove untrue declaration of Opera Mini (in reverse proxy mode) support from Drupal 9.3.

#129 works

@pivica, if you want to do the honors of adding the wording for #137 and #138, I'll be happy to review/RTBC.

regarding this issue,
I have been working with the module https://www.drupal.org/project/css_aspect_ratio

applying CSS aspect ratio allows for image dimensions being handled by CSS so the images require no explicit height and width other than set by image styles.

using this method, I have been able to achieve a score of 0 CLS with no reflow.

Here's the updated wording:

Fixed CS error.

Requeued a test run. Test failure in #150 is related to #3262505: [random test failure] Random error in layout_builder FunctionalJavascript tests.

Before reviewing, just putting out here that a compromise can also be to serve the smallest fallback image AND then use some JS shim to load a larger image -- most old browsers do support JS and such the responsive support can be emulated client side.

RTBC - The patch and approach looks great to me.

I have a question here, thanks for the patch that has worked great so far. I had to roll the patch after the update from Drupal 9.2 to 9.3, I noticed that something is not working correctly with an image style. in my specific example, it concerns the image style for logos, which is very quickly noticeable, especially on a 4k screen.

for logos i have the following image styles: 1x 100x100px, 2x 200x200px. for this i created a responsive image style, which uses the first image style for 1x and the second one for 2x. on a 4k monitor under drupal 9.2.x the logo was output in high res, so with 2x, but the visual size remained the same as on a non 4k screen.

since the last patch (#150), the logo on a 4k monitor is now output with 200x200px, so visually the logo is enlarged and appears very blurry. which is understandable, since you actually generated a size dimension of 200x200px. i have tried everything possible but it has all brought nothing. so i have first simply taken out the version 2x, so that the visual output of the logos is correct, even if it is now of course very blurry output on 4k resolution.

attached a screenshot that illustrates the problem well.

@zcht, can you share the rendered HTML for 9.2 (#25) vs 9.3 (##150?

@heddn good idea, attached both versions. it seems that in patch #150 width & height is changed, is also already in patch #129 exactly the same. that's why this "effect" occurs.

Drupal 9.2.x + patch #25

<a href="https://mywebsite.host/abc">     
  
  <div class="media media--blazy media--bundle--newsroom media--loading is-b-loading media--switch media--switch--content media--responsive media--image">
  
  <noscript>

  <picture>
    <source srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.webp?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.webp?itok=SJ6VoMFH 2x" media="all and (min-width: 1600px)" type="image/webp" width="100" height="100"/>
    <source srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.webp?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.webp?itok=rdX6a7e5 2x" media="all and (min-width: 1024px) and (max-width: 1599px)" type="image/webp" width="100" height="100"/>
    <source srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.webp?itok=s3QfTshE 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.webp?itok=rdX6a7e5 2x" media="all and (min-width: 900px) and (max-width: 1023px)" type="image/webp" width="100" height="100"/>
    <source srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.webp?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.webp?itok=rdX6a7e5 2x" media="all and (min-width: 600px) and (max-width: 899px)" type="image/webp" width="100" height="100"/>
    <source srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.webp?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.webp?itok=rdX6a7e5 2x" media="all and (max-width: 599px)" type="image/webp" width="100" height="100"/>
    <source srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.png?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.png?itok=SJ6VoMFH 2x" media="all and (min-width: 1600px)" type="image/png" width="100" height="100"/>
    <source srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.png?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.png?itok=rdX6a7e5 2x" media="all and (min-width: 1024px) and (max-width: 1599px)" type="image/png" width="100" height="100"/>
    <source srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.png?itok=s3QfTshE 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.png?itok=rdX6a7e5 2x" media="all and (min-width: 900px) and (max-width: 1023px)" type="image/png" width="100" height="100"/>
    <source srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.png?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.png?itok=rdX6a7e5 2x" media="all and (min-width: 600px) and (max-width: 899px)" type="image/png" width="100" height="100"/>
    <source srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.png?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.png?itok=rdX6a7e5 2x" media="all and (max-width: 599px)" type="image/png" width="100" height="100"/>

    <img class="media__image media__element" src="/sites/default/files/styles/myimagestyle/public/logo/logo.png?itok=uKDzrnac" width="100" height="100" loading="lazy" />
  </picture>

  </noscript>
    
  <picture>
    <source srcset="" media="all and (min-width: 1600px)" type="image/webp" width="100" height="100" data-srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.webp?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.webp?itok=SJ6VoMFH 2x"/>
    <source srcset="" media="all and (min-width: 1024px) and (max-width: 1599px)" type="image/webp" width="100" height="100" data-srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.webp?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.webp?itok=rdX6a7e5 2x"/>
    <source srcset="" media="all and (min-width: 900px) and (max-width: 1023px)" type="image/webp" width="100" height="100" data-srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.webp?itok=s3QfTshE 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.webp?itok=rdX6a7e5 2x"/>
    <source srcset="" media="all and (min-width: 600px) and (max-width: 899px)" type="image/webp" width="100" height="100" data-srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.webp?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.webp?itok=rdX6a7e5 2x"/>
    <source srcset="" media="all and (max-width: 599px)" type="image/webp" width="100" height="100" data-srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.webp?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.webp?itok=rdX6a7e5 2x"/>
    <source srcset="" media="all and (min-width: 1600px)" type="image/png" width="100" height="100" data-srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.png?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.png?itok=SJ6VoMFH 2x"/>
    <source srcset="" media="all and (min-width: 1024px) and (max-width: 1599px)" type="image/png" width="100" height="100" data-srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.png?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.png?itok=rdX6a7e5 2x"/>
    <source srcset="" media="all and (min-width: 900px) and (max-width: 1023px)" type="image/png" width="100" height="100" data-srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.png?itok=s3QfTshE 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.png?itok=rdX6a7e5 2x"/>
    <source srcset="" media="all and (min-width: 600px) and (max-width: 899px)" type="image/png" width="100" height="100" data-srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.png?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.png?itok=rdX6a7e5 2x"/>
    <source srcset="" media="all and (max-width: 599px)" type="image/png" width="100" height="100" data-srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.png?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.png?itok=rdX6a7e5 2x"/>

    <img class="media__image media__element b-lazy b-responsive" data-src="/sites/default/files/styles/myimagestyle/public/logo/logo.png?itok=uKDzrnac" src="data:image/svg+xml;charset=utf-8,%3Csvg%20xmlns%3D&#039;http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg&#039;%20viewBox%3D&#039;0%200%201024%201024&#039;%2F%3E" width="100" height="100" alt="abc" loading="lazy" />

  </picture>
  </div>
  
</a>

-------

Drupal 9.3.x + patch #150

<a href="https://mywebsite.host/abc">

  <div class="media media--blazy media--bundle--newsroom is-b-loading media--switch media--switch--content media--responsive media--image">

  <picture>
    <source srcset="" media="all and (min-width: 1600px)" type="image/webp" width="200" height="200" data-srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.webp?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.webp?itok=rdX6a7e5 2x"/>
    <source srcset="" media="all and (min-width: 1024px) and (max-width: 1599px)" type="image/webp" width="200" height="200" data-srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.webp?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.webp?itok=rdX6a7e5 2x"/>
    <source srcset="" media="all and (min-width: 900px) and (max-width: 1023px)" type="image/webp" width="200" height="200" data-srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.webp?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.webp?itok=rdX6a7e5 2x"/>
    <source srcset="" media="all and (min-width: 600px) and (max-width: 899px)" type="image/webp" width="200" height="200" data-srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.webp?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.webp?itok=rdX6a7e5 2x"/>
    <source srcset="" media="all and (max-width: 599px)" type="image/webp" width="200" height="200" data-srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.webp?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.webp?itok=rdX6a7e5 2x"/>
    <source srcset="" media="all and (min-width: 1600px)" type="image/png" width="200" height="200" data-srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.png?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.png?itok=rdX6a7e5 2x"/>
    <source srcset="" media="all and (min-width: 1024px) and (max-width: 1599px)" type="image/png" width="200" height="200" data-srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.png?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.png?itok=rdX6a7e5 2x"/>
    <source srcset="" media="all and (min-width: 900px) and (max-width: 1023px)" type="image/png" width="200" height="200" data-srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.png?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.png?itok=rdX6a7e5 2x"/>
    <source srcset="" media="all and (min-width: 600px) and (max-width: 899px)" type="image/png" width="200" height="200" data-srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.png?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.png?itok=rdX6a7e5 2x"/>
    <source srcset="" media="all and (max-width: 599px)" type="image/png" width="200" height="200" data-srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.png?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.png?itok=rdX6a7e5 2x"/>

    <img loading="lazy" class="media__image media__element b-lazy b-responsive" decoding="async" data-src="/sites/default/files/styles/myimagestyle/public/logo/logo.png?itok=uKDzrnac" src="data:image/svg+xml;charset=utf-8,%3Csvg%20xmlns%3D&#039;http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg&#039;%20viewBox%3D&#039;0%200%201024%201024&#039;%2F%3E" width="100" height="100" alt="abc" />
  </picture>

  <noscript>
    <picture>
      <source srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.webp?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.webp?itok=rdX6a7e5 2x" media="all and (min-width: 1600px)" type="image/webp" width="200" height="200"/>
      <source srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.webp?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.webp?itok=rdX6a7e5 2x" media="all and (min-width: 1024px) and (max-width: 1599px)" type="image/webp" width="200" height="200"/>
      <source srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.webp?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.webp?itok=rdX6a7e5 2x" media="all and (min-width: 900px) and (max-width: 1023px)" type="image/webp" width="200" height="200"/>
      <source srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.webp?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.webp?itok=rdX6a7e5 2x" media="all and (min-width: 600px) and (max-width: 899px)" type="image/webp" width="200" height="200"/>
      <source srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.webp?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.webp?itok=rdX6a7e5 2x" media="all and (max-width: 599px)" type="image/webp" width="200" height="200"/>
      <source srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.png?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.png?itok=rdX6a7e5 2x" media="all and (min-width: 1600px)" type="image/png" width="200" height="200"/>
      <source srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.png?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.png?itok=rdX6a7e5 2x" media="all and (min-width: 1024px) and (max-width: 1599px)" type="image/png" width="200" height="200"/>
      <source srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.png?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.png?itok=rdX6a7e5 2x" media="all and (min-width: 900px) and (max-width: 1023px)" type="image/png" width="200" height="200"/>
      <source srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.png?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.png?itok=rdX6a7e5 2x" media="all and (min-width: 600px) and (max-width: 899px)" type="image/png" width="200" height="200"/>
      <source srcset="/sites/default/files/styles/myimagestyle/public/logo/logo.png?itok=uKDzrnac 1x, /sites/default/files/styles/myimagestylex2/public/logo/logo.png?itok=rdX6a7e5 2x" media="all and (max-width: 599px)" type="image/png" width="200" height="200"/>

      <img loading="lazy" class="media__image media__element" decoding="async" src="/sites/default/files/styles/myimagestyle/public/logo/logo.png?itok=uKDzrnac" width="100" height="100" />
    </picture>
  </noscript>        
  
  </div>
</a>

While digging into #157, I think I also found a partial solution to #134. Or at least, we now have test coverage for both of those flagged items. Let's see how this fares.

let's hope it will not break working solutions using CSS aspect ratio

Drupal 9.3.5

Patch #158 in a VIEW block -> Lazy loading attribute EAGER is selected for resposive Media Images, but Images stays defined as LAZY.

(Block as bootstrap 4 carousel)

Patch #158 works fine for me, thanks for that. i can also confirm the observation of comment #160.

NW for #160.

I cannot reproduce #160/#161. Did you perhaps miss the loading options in views? See attached screenshots of views configuration:

Configured as Eager:

Rendered as Eager:

My mistake, @heddn is right in comment #163. everything works with the latest patch as requested and perfect with drupal 9.3.5, thanks again for that :)

RTBC++ for patch #158

Great work everyone. Lighthouse tests are much better and the option to switch in view modes from Eager to Lazy gives you all the flexibility you need. RTBC + 1

1. +++ b/core/modules/responsive_image/responsive_image.module
   @@ -365,7 +379,7 @@ function _responsive_image_build_source_attributes(array $variables, BreakpointI
      $derivative_mime_types = [];
   -  foreach ($multipliers as $multiplier => $image_style_mapping) {
   +  foreach (array_reverse($multipliers) as $multiplier => $image_style_mapping) {
        switch ($image_style_mapping['image_mapping_type']) {
   

   Could use a comment as to why we use array_reverse()

2. +++ b/core/modules/responsive_image/responsive_image.post_update.php
   @@ -13,3 +13,24 @@ function responsive_image_removed_post_updates() {
   + */
   +function responsive_image_post_update_image_loading_attribute() {
   +  $storage = \Drupal::entityTypeManager()->getStorage('entity_view_display');
   +  foreach ($storage->loadMultiple() as $view_display) {
   +    $changed = FALSE;
   +    /** @var \Drupal\Core\Entity\Display\EntityViewDisplayInterface $view_display */
   +    foreach ($view_display->getComponents() as $field => $component) {
   +      if (isset($component['type']) && ($component['type'] === 'responsive_image')) {
   +        $component['settings']['image_loading']['attribute'] = 'lazy';
   +        $view_display->setComponent($field, $component);
   +        $changed = TRUE;
   +      }
   +    }
   +    if ($changed) {
   +      $view_display->save();
   +    }
   +  }
   

   The manipulation of the views should happen in presave of views config entities - lots of examples in views post updates and config listeners where similar things happen. The update can then just use ConfigEntityUpdater (which would also help with the lack of batch here).

3. +++ b/core/modules/responsive_image/src/Plugin/Field/FieldFormatter/ResponsiveImageFormatter.php
   @@ -148,6 +153,27 @@ public function settingsForm(array $form, FormStateInterface $form_state) {
   +      '#title' => $this->t('Image loading'),
   +      '#weight' => 10,
   +      '#description' => $this->t('Image assets are rendered with native browser loading attribute of (<em>loading="lazy"</em>) by default. This improves performance by allowing modern browsers to lazily load images without JavaScript. It is sometimes desirable to override this default to force browsers to download an image as soon as possible using the "<em>eager</em>" value instead.'),
   +    ];
   

   Would be nice if we could make this a bit more concise somehow.

   Maybe 'Modern browser will lazily load images with the 'loading="lazy"' attribut (default) without JavaScript. Choose 'eager' to force browsers to download an image so soon as possible.'?

With responsive images https://validator.w3.org is complaining about :

• Error: Attribute width not allowed on element source at this point.
• Error: Attribute height not allowed on element source at this point.

While working on the feedback from #168, I found another bug. If there are breakpoints like:

• 1x
• 1.5x
• 2x

it sorts them:

• 1.5x
• 1x
• 2x

This means that the docs in the code that all breakpoints are sorted in a certain way cannot be trusted. Working on a fix for that and other review points.

Responded to #168. Will also be opening up a new issue (hopefully a quickfix) that will block landing this patch for the problem IDed in #170.

Opened #3267870: Order image mappings by breakpoint ID and numeric multiplier. And some fixes.

Resolved CS errors & added an interdiff.