WordPress.org

Make WordPress Core

Opened 3 months ago

Last modified 3 weeks ago

#53675 assigned enhancement

Omit lazy-loading attribute on first content image/iframe

Reported by: flixos90 Owned by: flixos90
Milestone: 5.9 Priority: normal
Severity: normal Version: 5.5
Component: Media Keywords: has-patch has-unit-tests
Focuses: performance Cc:

Description

Per this web.dev article, lazy-loading elements above the fold can lead to slight regressions in the Largest Contentful Paint metric. Since WordPress core by default lazy-loads every image and iframe, this also includes those in the initial viewport which has shown to impact LCP in the field at scale. After analyzing the situation of the current core implementation and the theme ecosystem (see this post), it is worth refining the default behavior of WordPress, in order to provide a solid default lazy-loading experience for the most common layouts (e.g. single column, hero image).

Related: #50425

Change History (20)

#1 @westonruter
3 months ago

  • Version set to 5.5

I have a POC patch in plugin form linked to at https://core.trac.wordpress.org/ticket/50425#comment:14

#2 @Mte90
3 months ago

Just because of thinking, the various website I manage with different themes and plugins.
I think that the best idea should be something like a global that track how many executions of (as example) wp_get_attachment_image and on first element doesn't add that parameter.
With adding a filter to disable that behaviour.

I know that a global is not the best thing but in this way is more granular in all the website without asking every developer from blocks to theme developers.

#3 follow-up: @flixos90
3 months ago

@Mte90 A global counter is what I was thinking too. I also agree this should be filterable - I'd argue in addition to turning it off it would be useful for themes with specific layouts where more than the first image typically appears above the fold.

@westonruter This is close to what I did in the prototype plugin used for the analysis - the one thing missing from yours (as you mention in the other ticket) was archive support. It would be great if you could review the prototype plugin (see below) and leave your thoughts.

For reference, this Gist has the prototype plugin with the logic used during the analysis:

  • Increase a counter for every content image (covering featured images and post content images).
  • If the first image is encountered, prevent the loading="lazy" attribute from being added.
  • This works for both single post views and archive views - in an archive, only the first image from the first post will not be lazy-loaded.

This plugin uses a function parameter to control how many images to omit from being lazy-loaded. For the core implementation, we could e.g. introduce a filter to allow modification. A theme that e.g. displays a grid archive with 3 columns could then set the number of images to not lazy-load for archives to 3.

Another thing to note here is that the plugin only focuses on images. The core implementation should also count iframes in the same way, in a common counter - i.e. the first image or iframe won't be lazy-loaded.

#4 @Mte90
3 months ago

So for iframe I think that we can do it for oembed (and blocks) but not for custom added inside the posts but should be enough I guess.

Usually a lot of plugins creates widget with iframe like mailchimp or instagram but usually they are used in the footer, so if we cover the oembed I think that is a acceptable percentage of usage.

#5 @azaozz
3 months ago

  • Milestone changed from Awaiting Review to 5.9

lazy-loading elements above the fold can lead to slight regressions in the Largest Contentful Paint metric

Sounds like something that needs to be fixed in the web browsers? They seem to be in a much better position to "see" (or guess) when an image or an iframe will be above the fold. Having said that, WP can also try to improve the situation until this is fixed in the browsers.

For reference, this Gist has the prototype plugin
...
For the core implementation, we could e.g. introduce a filter to allow modification.

Right. The Gist looks good but seems the code in core will be quite different once implemented.

Usually a lot of plugins create widget with iframes...

Yes, but thinking these plugins should be adding the loading attribute to their HTML themselves. Such plugins can usually detect when a widget is at the top, in the sidebar, or in the footer.

Looking at FSE/template parts, can probably use the same approach as with widgets: while editing add loading="eager" when the image or iframe is expected to be at the top/above the fold, and loading="lazy" when in the middle of the page or in the footer.

Lets look at this during 5.9.

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

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

Replying to flixos90:

@westonruter This is close to what I did in the prototype plugin used for the analysis - the one thing missing from yours (as you mention in the other ticket) was archive support. It would be great if you could review the prototype plugin (see below) and leave your thoughts.

Actually, my plugin does account for the archive template. My comment was referring to how twentytwentyone was not omitting loading=lazy from the first post's featured image on an archive template.

In your POC, you're stripping out loading="lazy" from the HTML of the featured image. Would it not be better to rather filter wp_lazy_loading_enabled? See alternative.

It's good how you're using wp_img_tag_add_loading_attr to account for markup in the content beyond just the featured image.

What if someone is adding an image to the template outside of the_content() and the_post_thumbnail()? They could also obtain an image via wp_get_attachment_image() directly. Since that function applies the wp_lazy_loading_enabled filter, perhaps the filter should increment the counter instead of in the post_thumbnail_html and wp_img_tag_add_loading_attr filters both?

This ticket was mentioned in PR #1580 on WordPress/wordpress-develop by felixarntz.


2 months ago

  • Keywords has-patch has-unit-tests added; needs-patch needs-unit-tests removed

This PR implements the refinement described in this post as a core enhancement.

  • Introduce a new function wp_get_loading_attr_default( $context ) that should be used in place of the hard-coded 'lazy' string defaults used throughout the codebase so far.
    • The function still returns 'lazy' in most circumstances.
    • However, for contexts the_content and the_post_thumbnail (new, see below) it returns false (to omit the loading attribute) for the very first image or iframe within the main query loop.
  • Introduce a new filter wp_omit_loading_attr_threshold in order to allow fine tuning of the default threshold of 1 content image or iframe (e.g. based on special layout usage by a theme). For example, if the filter is used to set the threshold to 3, the first 3 images or iframes won't be lazy-loaded.
  • A new context the_post_thumbnail is introduced, used within get_the_post_thumbnail() and thus taking precedence over the wp_get_attachment_image context that would be used by wp_get_attachment_image().
  • One rather minor, but significant change has been made to wp_filter_content_tags():
    • The function used to first go over all images and then all iframes.
    • However, since now sequence truly matters in order to decide whether or not to add a loading attribute, the function now goes over all the matches in their correct order. Nothing else has changed in the logic there, and no existing tests are affected by this change.
  • Test coverage is present.

Trac ticket: https://core.trac.wordpress.org/ticket/53675

#8 in reply to: ↑ 6 @flixos90
2 months ago

@azaozz @westonruter Would you mind reviewing / testing the PR https://github.com/WordPress/wordpress-develop/pull/1580?

Replying to westonruter:

What if someone is adding an image to the template outside of the_content() and the_post_thumbnail()? They could also obtain an image via wp_get_attachment_image() directly. Since that function applies the wp_lazy_loading_enabled filter, perhaps the filter should increment the counter instead of in the post_thumbnail_html and wp_img_tag_add_loading_attr filters both?

This question may now be slightly off in relation to the core implementation, however I wanna address the point of considering manually placed images via wp_get_attachment_image(). It may be worth thinking about this, but so far I'm thinking that the use-case of manually rendered images within the content is too insignificant, plus themes may use this to e.g. render small icon-like images etc., considering which here would be rather counter-productive. Of course the post content could also contain such tiny icon-like images as the first elements, but I think that's a lot less likely.

I think relying on only featured images and in-content images is more reliable here.

#9 @prbot
2 months ago

mtias commented on PR #1580:

@felixarntz have you thought about how we could have more precise heuristics going forwards that can take the semantics and structure of blocks into account to get a sense for what is actually deferrable? For example, an image block or a site logo used in a header template part would be strong indicatives of being above the fold. "The first image of the content" seems instead like a rudimentary measure, that varies a lot depending on preceding layout. With block themes we should have some ahead-of-time awareness of layout which we can use to produce more meaningful instructions.

#10 @prbot
2 months ago

felixarntz commented on PR #1580:

@mtias Great point, there is definitely room for further refinement as we're moving towards a world of block themes. I've been thinking about detecting the header template part as well.

The analysis I've conducted using the fix here (even though technically I agree that "the first content image" _sounds_ rudimentary) has proven that it already significantly improves performance across the board, so I think it's a useful enhancement on its own - I don't think we should overcomplicate it for now. Once there are more block-based themes available, I agree it would make sense to refine further by using the semantics of blocks. For example, we could also make decisions like "if the first block is a gallery block with three images per row, omit lazy-loading on the first _three_ images".

#11 @prbot
2 months ago

westonruter commented on PR #1580:

Also wanted to note that I found that 3,801 themes (out of 4,145) make use of the main element. I was surprised (and heartened) to see that 90%+ of themes use this semantic element. For the AMP plugin, we've used this as an easy way to identify all images that appear in the header, by looking for all that occur before the `main`. Granted, this wouldn't work the same way in core since the DOM of the page is not available, but eventually in a FSE context this could be implemented similarly through filtering of blocks.

#12 @prbot
2 months ago

mtias commented on PR #1580:

@felixarntz yes, I didn't mean rudimentary in a disparaging way, just that it feels we could be doing more tailored targeting there with block semantics.

#13 @prbot
4 weeks ago

adamsilverstein commented on PR #1580:

Testing with twentynineteen, the header/top image seems to still get the lazy attribute, while for content's first image which you have to scroll to reach does not get it.

https://i0.wp.com/user-images.githubusercontent.com/2676022/134570815-e8c94b11-345b-4032-8d05-e24cde9926f4.png

#14 @prbot
4 weeks ago

adamsilverstein commented on PR #1580:

verified working correctly in twentytwenty so must be something specific about how 2019 outputs that top image.

#15 @prbot
3 weeks ago

felixarntz commented on PR #1580:

@adamsilverstein

Testing with twentynineteen, the header/top image seems to still get the lazy attribute, while for content's first image which you have to scroll to reach does not get it.

That was expected behavior with the proposed fix, since that theme is rendering the featured image outside of the loop. It raises a good point though that maybe we should support that case, since it is fairly common to render the featured image on top of the page when is_singular(). I've implemented this in https://github.com/WordPress/wordpress-develop/pull/1580/commits/03a95a93ecb0b267533e8bfb5825e690541fe152. Similar to the other changes in this PR, this is not 100% reliable, but it won't have adverse effects, and I'm assuming it will bring even better LCP results than without it.

I will run a separate follow-up A/B test to compare. Until then, happy to hear your feedback @westonruter on this new commit as well.

#16 @prbot
3 weeks ago

felixarntz commented on PR #1580:

@adamsilverstein @westonruter @thierryA I've ran a test between both versions of the fix (see above https://github.com/WordPress/wordpress-develop/pull/1580#issuecomment-931592700) across the 30 most "popular" themes based on the wordpress.org API. To clarify, the two different versions are:

  • "corefix1" counts all the_content and the_post_thumbnail images as long as they're also in_the_loop().
  • "corefix2" is based on the latest iteration here, so it supports one extra condition where the_post_thumbnail is also counted for a "singular" context, even if not in_the_loop() (where it e.g. covers the special behavior in Twenty Nineteen).

The difference between the two is barely noticeable:

  • For both LCP and image KB loaded, the median difference is 0%.
  • Especially for LCP there are no notable wins on either end, although the number of tests where there is an improvement in "corefix2" is slightly higher than the number of tests with an improvement in "corefix1". That isn't statistically sufficient for a decision though.
  • In image KB, there is barely any difference, in most individual tests the difference is 0%. It's in fact only 4 themes where any scenario gave a result other than 4%, and two of those are indeed Twenty Nineteen and Twenty Seventeen, which both for a singular post have that special behavior that they display the featured image outside of the loop.

Given that there is no win across the board from also counting featured images in singular context outside the loop and since it is also a special case anyway, I suggest we revert https://github.com/WordPress/wordpress-develop/pull/1580/commits/03a95a93ecb0b267533e8bfb5825e690541fe152 again and go with the original approach. I don't think adding a special case just for some core themes with a non-standard implementation is justifiable. Thoughts?

#17 @prbot
3 weeks ago

adamsilverstein commented on PR #1580:

Given that there is no win across the board from also counting featured images in singular context outside the loop and since it is also a special case anyway, I suggest we revert 03a95a9 again and go with the original approach. I don't think adding a special case just for some core themes with a non-standard implementation is justifiable. Thoughts?

I agree your code here should be as generalized as possible and you should revert the final change. One thing to consider would be changing twentynineteen itself since we maintain that in core as well and I'm guessing it is still in very wide usage.

#18 @prbot
3 weeks ago

felixarntz commented on PR #1580:

@adamsilverstein Reverted!

We can look at changing Twenty Nineteen as well, but it might be tricky. While it should be possible to move that header content into the loop without affecting the actual design, a problem would be child themes. For example a child theme that overrides header.php and nothing else would then most likely see that image twice because they would still use the template based on the old version of the theme.

#19 @prbot
3 weeks ago

ThierryA commented on PR #1580:

Given that there is no win across the board from also counting featured images in singular context outside the loop and since it is also a special case anyway, I suggest we revert 03a95a9 again and go with the original approach. I don't think adding a special case just for some core themes with a non-standard implementation is justifiable. Thoughts?

Sounds totally reasonable to me, +1!

#20 @prbot
3 weeks ago

azaozz commented on PR #1580:

I'm actually having some "second thoughts" on this. As far as I understand this is a browser problem. The browsers should be able to detect when an image s going to be in the viewport and load it eagerly. That's in the specs.

It is somewhat unfortunate that this doesn't currently work particularly well as seen by @felixarntz's post, but I'm not 100% sure WordPress should be trying to partially remedy the situation. No matter what we do it will always be a partial, hit-and-miss solution as there's no chance to guess the viewpors from the server. The browsers are much better equipped for that and should be fixing it.

Another thing is that WordPress already slows things down pretty considerably by implementing srcset and sizes and loading attributes on-the-fly at the front-end. Making many billions of page loads on many millions of sites even a bit slower is a hefty price to pay for a somewhat partial improvement.

Thinking it would be better to see if the browsers fix this and in case they don't/won't perhaps try to add some code on the server. My suggestion would be to come back to this in 6 months to a year and asses it again.

Note: See TracTickets for help on using tickets.