WordPress.org

Make WordPress Core

Opened 8 months ago

Last modified 4 weeks ago

#53232 new feature request

Add async decoding attribute as default to the HTML image tags generated by WordPress

Reported by: isaumya Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Media Keywords: has-patch has-unit-tests
Focuses: performance Cc:

Description

Hi,
WordPress has taken advantage of many amazing features to the image tags like srcset, sizes and recently loading attributes. Thanks to the loading="lazy" attribute all images generated by WordPress system have that attribute in it by default. This is insanely helpful to make page load faster and more responsive.

Recently I have been playing around with another special attribute to the img tag which is basically decoding="async". After implementing async decoding to the images in a page, the page load became, even more, faster and images are decoded asynchronously by the browser, loading the contents almost instantly and also reducing page render time. This is a huge performance booster to any webpages which has a lot of images (so basically most sites).

This decoding attribute also has great browser support according to MDN docs.

While trying to implement the feature on a WordPress site, with the help of the following code snippet I was able to make it work across all the images on the website except for the images added via Gutenberg.

<?php
add_filter( 'wp_get_attachment_image_attributes', function($attr) {
  $attr['decoding'] = 'async';

  return $attr;
} );

As Gutenberg doesn't support either image_send_to_editor filter or wp_get_attachment_image_attributes filter. Adding it to Gutenberg generated images are more complicated and includes overwriting many core blocks like images, gallery etc.

But I was wondering if the WP Core team consider adding this decoding="async" attribute to the images like loading="lazy" as it will help a lot of people, making their webpage render faster by browsers.

Attachments (1)

53232.1.diff (23.6 KB) - added by westonruter 3 months ago.

Download all attachments as: .zip

Change History (32)

#1 @westonruter
8 months ago

Thanks for filing this. I was actually asking if anyone has raised this yet and I was going to file it, but then you beat me to it.

The use of decoding=async is deemed a best practice to according to Malte Ubl:

Adding decoding="async" to the img gives the browser permission to decode the image off the main thread avoiding user impact of the CPU-time used to decode the image. This should have no discernible downside except that it cannot always be the default for legacy reasons.

I'm not clear on the specifics of the legacy reasons. I think it may have to do with if you try to load an image with JavaScript and then try to access its pixels. With async decoding, you then have to use the decode method to get a promise for when it is decoded.

It's also a best practice according to Addy Osmani.

In AMP all images also get decoding=async for the performance benefit.

In Next.js, the Image component uses decoding=async.

I don't see any downsides to adding decoding=async to all images, even those that we don't currently add loading=lazy. Basically, anywhere that a srcset can be supplied we can also inject decoding=async.

#2 @isaumya
8 months ago

You are absolutely right @westonruter. Haha! It's funny, I also asked in the Slack core channel if anyone has a trac ticket open about decoding="async" and no one replied to me. So, I thought, let's open one, and if there is already an existing ticket about this, at most it will get closed/merged with that ticket. I even tweeted about it.

I'm not clear on the specifics of the legacy reasons. I think it may have to do with if you try to load an image with JavaScript and then try to access its pixels. With async decoding, you then have to use the decode method to get a promise for when it is decoded.

I think you are right. But this still should be the default, as there is no easy way to add it to Gutenberg generated images as I stated above.

I'm not clear on the specifics of the legacy reasons. I think it may have to do with if you try to load an image with JavaScript and then try to access its pixels. With async decoding, you then have to use the decode method to get a promise for when it is decoded.

I think it would be beneficial even if there is a single image and no srcset and a single image url. Don't you think?

#3 follow-up: @ayeshrajans
8 months ago

This will be a nice one, thank you for opening the ticket.

I took a quick look at how lazy loading is currently implemented, and I think we should follow suit:

  • Add wp_img_tag_add_decoding_attr filter akin wp_img_tag_add_loading_attr to determine offer attribute value.
  • Add wp_img_tag_add_decoding_attr function akin wp_img_tag_add_loading_attr to make the actual change.
  • Update wp_filter_content_tags to call wp_img_tag_add_decoding_attr.
  • Make sure kses allow-lists the decoding attribute.
  • Add test coverage.

I would suggest to only add the decoding attribute only if loading=lazy is also present. This might(?) reduce sudden content reflows.

#4 @isaumya
8 months ago

Hi @ayeshrajans,
Thanks for the +1. I Googled about those filter you mentioned but couldn't find any documentation about it. Do you have any snippet to currently add it in the Gutenberg generated images? As the code block I provided above won't work for any Gutenberg images and there is no image_send_to_editor for Gutenberg, so I couldn't add it for Gutenberg generated pages.

If you have a working code snippet that will help a lot of people to add it right now to their WP sites until this gets added to the core code.

#5 in reply to: ↑ 3 ; follow-up: @westonruter
8 months ago

Replying to ayeshrajans:

Make sure kses allow-lists the decoding attribute.

This wouldn't be necessary since the decoding attribute is added on the frontend and is not stored in the post content. The loading attribute is not stored in post content either. Nevertheless, I see loading was explicitly added to Kses so might as well add it.

I would suggest to only add the decoding attribute only if loading=lazy is also present. This might(?) reduce sudden content reflows.

I don't believe this is necessary. I believe the layout shifting issue is only relevant to lazy-loading.

I took a quick look at how lazy loading is currently implemented, and I think we should follow suit:

Otherwise, your proposal for how to implement seems good to me!

Replying to isaumya:

If you have a working code snippet that will help a lot of people to add it right now to their WP sites until this gets added to the core code.

This will do the trick for all images in the post content:

<?php
add_filter(
        'the_content',
        static function ( $content ) {
                return str_replace( '<img ', '<img decoding=async ', $content );
        }
);

#6 in reply to: ↑ 5 @isaumya
8 months ago

Thanks a ton, @westonruter for the snippet. Actually, I was trying to avoid using the_content filter and instead was looking for a way to directly hook into the images rather than parsing the whole content. As when using the_content if the content is very large, PHP has to do really a lot of work executing and parsing it. I just thought using the_content was not an optimized code.

#7 @westonruter
8 months ago

Well, using the_content is how the loading attribute is added. It's how core adds srcset and sizes as well. The performance impact should be negligible. True it will be faster when when/if the replacement is done inside of wp_filter_content_tags() since there is less content to search through, but in the meantime a simple the_content filter will get you decoding=async now.

#8 @westonruter
8 months ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 5.8

Another explanation on the benefits of decoding=async from Matt Fantinel:

decoding="async" tells your browser it can try to parallelize loading your image. When your page is loading, it tries to decode both text and images at the same time. On lower-end devices though, decoding heavy images can take a while, and this might block the rendering of the rest of the content. With this option, the browser will try to proceed rendering the rest of the content and render the image later. This can be a great improvement to perceived performance.

I'm going to milestone this for 5.8 even though we're nearing the May 25th date for the feature freeze, although the patch should be small. This will probably get punted to 5.9.

#9 @westonruter
8 months ago

I'm checking with some colleagues who focus on web performance and it may actually be preferable to not add decoding=async to images in the first viewport. So it may indeed be that decoding=async should only be added if loading=lazy is also being added.

#10 @westonruter
8 months ago

OK, I checked with Malte Ubl and he says:

Another highly relevant nuance:
decoding=async does ~nothing on a reasonably sized image, but it is impactful on the occasional 20 Megapixel outlier. That page will still load slow but the UI thread won't be impacted.

If you know that you're appropriately sizing images, then it doesn't really matter what value the decoding attribute has. But if you don't know (e.g. you are WordPress and you unfortunately don't have image optimization build in), then async decoding helps prevent outlier bad performance while having effectively no impact on the average page.

Therefore, this does seem relevant to add to all images in WordPress since it is relatively common for oversized images to be served, where image smaller sizes may fail to be generated for the srcset. Granted, the chance of a 20MB image being added to a post is greatly reduced since WordPress 5.3 improved handling of big images. Nevertheless, I'm not sure the big image handling is always working. I just tried adding a 90MB image post and it was added to the post without reduced size or other image sizes being created. There probably wasn't enough memory available to do any manipulation of the image at all. So this shows that decoding=async is a useful protection to have enabled by default in WordPress.

#11 @isaumya
8 months ago

Thanks a ton, @westonruter for all the in-depth analysis on this matter. It seems more impactful than I initially thought, for most WordPress sites especially the sites using page builders. Improper image size is super common in WP page builder world.

#12 @westonruter
8 months ago

  • Milestone changed from 5.8 to Future Release

#13 @isaumya
8 months ago

Hey @westonruter, so 5.9 or maybe later to that?

#14 @westonruter
8 months ago

Yeah, 5.9. The feature freeze for 5.8 is tomorrow, so it seems too late to include for this release.

#15 @westonruter
7 months ago

Aside: HTTP Archive just added tracking for usage of the decoding attribute on images: https://github.com/HTTPArchive/legacy.httparchive.org/pull/206

#16 @westonruter
6 months ago

  • Milestone changed from Future Release to 5.9

This makes sense to do in 5.9 alongside #53675.

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


3 months ago

#18 @antpb
3 months ago

If #53675 is landed for 5.9 and review is needed on this ticket please feel free to reach out to me and I can prioritize with the media team! Thanks!

#19 @isaumya
3 months ago

Hey @westonruter,
One minor thing to add here. As this ticket is being considered with the combination with the not lazy loading first image ticket that's why saying.

I think for the first image that is not being lazy-loaded we should also have importance="high" as from Chrome 95 it is fully supported in the native browsers. Also if someone is using < Chrome 95 it will not cause any issues.

What do you think?

#20 @westonruter
3 months ago

@isaumya That's a good question. First of all, Priority Hints aren't yet standardized (see draft spec and Chrome Platform status) so I think it's a bit premature to add the the importance attribute to non-lazy images; it's only supported by Chrome and not any other browsers.

But secondly, I think adding importance should probably be the domain of themes (or FSE blocks) which can know for sure whether an image is in the first viewport and it is an important image. It's one thing to omit the loading=lazy attribute to let the browser decide whether to lazy-load, but it's another thing to add the importance=high attribute to tell the browser to PLEASE DOWNLOAD THIS NOW when we actually aren't 100% certain based on our heuristics for whether it will be in the first viewport.

#21 @isaumya
3 months ago

Hi @westonruter,
Yes, I agree with you on that. I thought for the above the fold images you guys are gonna add loading="eager" instead of ommiting it. Sorry my misunderstanding.

Generally, when I build custom themes for custom development I always add the loading="eager" importace="high" for the first above the fold image that is going to general the LCP value.

But thanks a lot for your reply. I am hoping (kind of sure) that the importance attribute will be standardized very soon. I think Firefox will do it first and hopefully, Safary will either not do it or will do it after 5-10 years or so. As the other chromium-based browsers will have it (I think) as they are chromium based.

Last edited 3 months ago by isaumya (previous) (diff)

#22 @westonruter
3 months ago

@isaumya It's actually not a best practice to add loading=eager to the first image, and to instead opt to just omit the loading attribute altogether. The reason for this is that in Chrome, for example, omission of the attribute implies that the browser should decide whether to lazy-load (e.g. based on the network connection). However, if you add loading=eager then it will download it unconditionally. See caution in Browser-level lazy-loading for CMSs:

If an element should not be lazy-loaded, require or encourage skipping the loading attribute entirely. While using loading="eager" is a supported alternative, this would tell the browser explicitly to always load the image right away, which would prevent potential benefits if browsers implemented further mechanisms and heuristics to automatically decide which elements to lazy-load.

#23 @isaumya
3 months ago

Yes @westonruter. But in many cases when you know for sure that whether the user is gonna visit the site on a mobile device or desktop that image is going to be the first image that they will see and cause the LCP. Doing this improves the LCP a lot as otherwise, the browser might download some other images below and then download the top image increasing the LCP. But yes I do agree with you it should not be used without knowing what exactly it is going to do and its consequences.

@westonruter
3 months ago

#24 @westonruter
3 months ago

  • Keywords has-patch added; needs-patch removed

I've drafted a patch.

I based it off of the pull request for #53675, so I haven't opened a PR yet since it is not yet committed.

Commit on GitHub: https://github.com/westonruter/wordpress-develop/commit/61d904c1f4e48363d4d2622cc47b8e09c611a75c

Note that most of the test changes are to make testing work on more environments, specifically where the wp-content directory is not in the standard location.

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


2 months ago

#26 @hellofromTonya
2 months ago

With [52065] now in core, @westonruter do you have time to finish the PR today? (i.e. 5.9 feature freeze is today)

This ticket was mentioned in PR #1853 on WordPress/wordpress-develop by westonruter.


2 months ago

  • Keywords has-unit-tests added

#28 follow-up: @westonruter
2 months ago

  • Milestone changed from 5.9 to Future Release

Punting this as we'll need to have more discussion than about this and do testing for what the defaults are, namely whether decoding=async should be on images in the first viewport where we now omit loading=lazy. We'll need to do more testing to see the impact.

#29 in reply to: ↑ 28 ; follow-up: @michaelbourne
4 weeks ago

Replying to westonruter:

We'll need to do more testing to see the impact.

What kind of testing did you have in mind? Happy to help here as I greatly want to see this added to core. I also saw no downsides in my tests with adding it to all images, but willing to deep dive it if needed.

#30 @isaumya
4 weeks ago

Hi @michaelbourne,
Yes, I also use the decoding="async" attribute to all image tags and have not faced any problems. Honestly, I also couldn't wait to see this being part of the core. But don't know when that will happen. I thought it's gonna be added to v5.9 but it seems it is not.

#31 in reply to: ↑ 29 @westonruter
4 weeks ago

Replying to michaelbourne:

What kind of testing did you have in mind? Happy to help here as I greatly want to see this added to core. I also saw no downsides in my tests with adding it to all images, but willing to deep dive it if needed.

Probably showing how specifically decoding=async improves performance in results from WebPageTest, showing before/after.

Note: See TracTickets for help on using tickets.