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

This snippet was already added to the 10.1.0 release notes draft.

Is anyone else finding that https://www.drupal.org/files/issues/2022-09-30/3192234-228-9.5.x.patch → is not applying to 10.0.3? I am including https://www.drupal.org/files/issues/2022-05-19/3267870-9.5.x-47.patch → first.

Hi @darren.fisher!
The patch is meant for D9 so you won't be able to apply it on D10.
You can try to apply the patch extracted from the commit but it might not work either as it's been done for D10.1 and not D10.0

Still no patch for D10.0?

I think it is because it landed in 10.1.

The merge request diff kinda applies but fails on class not found errors because it assumes they are already there from other update functions.

Uploading a D10 patch and also adding patch instructions here and in the issue summary. I hope we can finally let this issue rest in piece with that :)

"patches": {
  "drupal/core": {
    "Order image mappings by breakpoint ID and numeric multiplier": "https://www.drupal.org/files/issues/2022-05-19/3267870-9.5.x-47.patch",
    "3192234: Apply width and height attributes to responsive image tag": "https://www.drupal.org/files/issues/2023-04-07/3192234-293-10.0.x.patch",
  },
}

Big thanks for D10 patch @Berdir, I am confident multiple people were holding with D10 migration due to lack of this patch/feature on 10.0

#293 sadly doesn't seem to apply on 10.0.8 anymore.

Google Pagespeed is complaning about missing width and height attributes on the fallback image now.

The commit contained these lines:

+    // We don't set dimensions for fallback image if rendered in picture tag.
+    // In Firefox, it results in sizing the entire picture element to the size
+    // of the fallback image, instead of the size on the source element.
+    $dimensions = responsive_image_get_image_dimensions($responsive_image_style->getFallbackImageStyle(), [
+      'width' => $variables['width'],
+      'height' => $variables['height'],
+    ],
+      $variables['uri']
+    );
+    $variables['img_element']['#width'] = $dimensions['width'];
+    $variables['img_element']['#height'] = $dimensions['height'];

In the issue summary and comment #15 I found

Firefox has open issue: https://bugzilla.mozilla.org/show_bug.cgi?id=1694741 (linked from https://github.com/mozilla/standards-positions/issues/485)

when looking for "Firefox".
Both are closed.

Pagespeed says:

Image elements do not have explicit width and height
Set an explicit width and height on image elements to reduce layout shifts and improve CLS

for some <img ... />'s within picture tags.

So should we add a follow-up to investigate on removing this exclusion?

good catch! this sounds like a really important change

Perhaps those are thoughts that could be carried over into ✨ Add fallback format support to responsive images Needs work .

In what cases does this happen, because, on 9.5.x, the fallback image gets the correct parameters (width and height are set) set when using the patched version?

@KlemenDEV: That sounds strange, as the code in #296 is from the patch in #293 and I also checked the commit into 10.1.x and it has this code present.

Could someone else please verify this and provide a screenshot?

@heddn re #298 after thinking about this for a while I think we shouldn't mix that.

Once #299 has been clarified, a separate issue should be added to discuss the removal.

So to clarify #300 (we are running 9.5.x with "patch for Drupal 9" from the top of this issue), here are the settings:

and here is the code generated

@KlemenDEV which patch exactly? Could you provide the composer patches lines? This still doesn't make a lot of sense to me from the code perspective.

Here are the patches applied to the core:

"drupal/core": {
                "#3267870": "https://www.drupal.org/files/issues/2022-05-19/3267870-9.5.x-47.patch",
                "#3192234": "https://www.drupal.org/files/issues/2022-09-30/3192234-228-9.5.x.patch"
            },

@KlemenDEV thanks, you should update to the patches from #293 propably to have the same results as the commit for 10.1.x! This explains the differences - the older patch didn't have these lines which were merged into 10.1.x

So here's the follow-up: 📌 Apply width and height attributes to allow responsive image tag use loading="lazy" Fixed to remove this extra-handling! :)

Could someone a patch for 10.0.x from #293 that does not remove the width and height on the fallback image? I feel the amount of changes is too big for me to feel comfortable doing it

Ok, I have tried to do it myself, so here is a patch for 10.0.x that sets the width and height of the fallback image too.

Thank you @KlemenDEV but this issue is closed and that was already done in: 🐛 (Re-)Add width / height also on fallback image Fixed
but perhaps you'd like to review the MR over there?

I can once I get 10.1 dev env setup for the websites I can test with.

I know this is closed, I just wanted to provide a patch for people that are using 10.0.x with patch and want to have width/height set too.

Sorry for the ping everyone, but we opened a follow-up issue where I'd love to get feedback on.

In short, somewhere between the D9 and D10 patches there was a change on how the width/height works and at least for us, that is causing issues for the design. We have a fairly simple patch that changes this and it mostly works for us but any kind of feedback would be useful whether that's correct or if there's another way: 🐛 Responsive image width/height values are not used from fallback image style Needs work .

re #312: that might have been introduced by 🐛 (Re-)Add width / height also on fallback image Fixed .

xjm → credited attiks → .

xjm → credited Bowevil → .

xjm → credited chananiel → .

xjm → credited ekl1773 → .

xjm → credited Eric Heydrich → .

xjm → credited Jelle_S → .

xjm → credited jofitz → .

xjm → credited pixelite → .

xjm → credited RainbowArray → .

Adding a bunch of missed credits from a duplicate of this issue circa 2015-2017 (plus triage credits for @quietone and @andypost).