#50425 closed defect (bug) (fixed)
Do not lazy-load images which are very likely to be in the initial viewport
Reported by: | flixos90 | Owned by: | flixos90 |
---|---|---|---|
Milestone: | 5.5 | Priority: | normal |
Severity: | normal | Version: | 5.5 |
Component: | Media | Keywords: | has-patch has-unit-tests has-dev-note |
Focuses: | Cc: |
Description
Currently, loading="lazy"
is added to the following images by default:
- images in post content (context
the_content
) - images in post excerpt (context
the_excerpt
) - images in widget content (context
widget_text_content
) - avatar images (context
get_avatar
) - any image rendered via
wp_get_attachment_image()
(contextwp_get_attachment_image
)
While these defaults can be modified per context via the wp_lazy_loading_enabled
filter, there are certain images in WordPress which preferably would not be lazy-loaded since they are very likely to be in the initial viewport. This ticket aims at modifying those images to use loading="eager"
by default instead.
The following functions should pass loading="eager"
to wp_get_attachment_image()
:
get_custom_logo()
get_the_post_thumbnail()
Another function to cover would be get_header_image_tag()
, however that does not use wp_get_attachment_image()
, so no need to update.
Change History (16)
#2
@
4 years ago
My understanding is that modern browsers which implement native lazy loading would handle this decision making. ie. an image within the initial viewport could have the lazy attribute but the browser would still choose to load it immediately.
Is there a negative impact of including the lazy attribute on an image above the fold?
#3
in reply to:
↑ 1
;
follow-up:
↓ 14
@
4 years ago
Replying to joemcgill:
get_the_post_thumbnail()
is used extensively in places where the featured image is not in the viewport, e.g., on archive pages or in places where some UI is being creating using a secondary query, so we might want to be more explicit in that case that it only loads whenis_single()
andis_main_query()
are both true.
That's a fair point. I'd assume in the majority of cases the image would be somewhere near the top of the page, but it's true we cannot rely on that. I think it would make most sense to provide essential support for the lazy-loading and rely on themes to manage the specifics for these kinds of images, based on where they place them. I'll post a follow up comment with more details on the proposed solution.
Replying to johnbillion:
My understanding is that modern browsers which implement native lazy loading would handle this decision making. ie. an image within the initial viewport could have the lazy attribute but the browser would still choose to load it immediately.
Is there a negative impact of including the lazy attribute on an image above the fold?
Essentially, the loading="lazy"
attribute tells browsers to load the image lazily. While browsers may use some heuristics to assess priority of these images etc., having an image in the initial viewport marked with loading="lazy"
will result in a slightly different behavior than having it load eagerly, which can have an impact on the largest contentful paint result for the page. Essentially the browser will typically render the page without loading the image first, which usually happens in a split second, but does technically result in a slight delay.
We should be setting sane defaults in core, and then defer to theme (and in some instances plugin) developers for tweaking with lazy-loading in a more fine-grained way based on how they use images. I'll post the proposed approach in a follow-up comment.
#4
@
4 years ago
After re-thinking the approach outlined in the original ticket description, also taking into account the comments so far, I'm proposing to handle lazy-loading in the following way to achieve a good balance between saving network resources/decreasing initial load and maintaining a solid largest contentful paint outcome:
- Globally add support for the
loading
value to be specified asnull
, to explicitly omit the attribute. At this point, this is the recommended approach for images that should not be lazy-loaded - in other words, we should avoid usingloading="eager"
only to override a value oflazy
. - Lazy-load images by default only if they are within
the_content
,the_excerpt
, orwidget_text_content
, as well asget_avatar
images, as these areas are primarily found below the fold. - Support lazy-loading images used in templates via
wp_get_attachment_image
, but leave the attribute off by default. - Document in the dev-note the default behavior and how theme developers can modify it...
- ...by using the
wp_lazy_loading_enabled
filter to generally enable or disable lazy-loading for all images for a certain context (e.g. a theme could globally enable lazy-loading for template images or globally disable lazy-loading for widget content). This could be used by a plugin or theme developer, however it is rather general, so options 2. or 3. are preferable. - ...by explicitly lazy-loading certain template images via e.g.
wp_get_attachment_image( $id, $size, $icon, array( 'loading' => 'lazy' ) )
orget_the_post_thumbnail( $post, $size, array( 'loading' => 'lazy' ) )
. This could be used e.g. by a theme developer to lazy-load a featured image or logo used in the footer. - ...by disabling lazy-loading for a specific image via the
wp_img_tag_add_loading_attr
filter. This allows very fine-grained modification that in certain cases _may_ make sense for a theme to do, but would likely apply more to site-specific customization e.g. by an agency.
- ...by using the
This ticket was mentioned in PR #353 on WordPress/wordpress-develop by felixarntz.
4 years ago
#5
- Keywords has-patch has-unit-tests added; needs-patch needs-unit-tests removed
Trac ticket: https://core.trac.wordpress.org/ticket/50425
#7
@
4 years ago
- Type changed from enhancement to defect (bug)
- Version set to trunk
My understanding is that modern browsers which implement native lazy loading would handle this decision making.
Same here. Even if the browsers aren't super good at that at the moment, I'm pretty sure they will improve over time :)
Looking at https://github.com/WordPress/wordpress-develop/pull/353, most of it makes sense. The only thing that would perhaps need a confirmation if it is buggy is this: https://github.com/WordPress/wordpress-develop/pull/353/files#diff-67a565933b9d6a2850abd1d7dc5585d5R1618 (do not lazy load images that come from wp_get_attachment_image()
by default).
In that terms thinking the PR should be added to trunk now except the above change. If it is confirmed as a bug during beta testing, lets add that too (changing ticket type to bug as that's what it is, essentially).
#8
@
4 years ago
I tend to agree that the browsers should be deciding exactly how and what to lazy load, ie. images in the viewport at load should load immediately regardless of a lazy attribute. It would be interesting to see some data about how impactful this is.
I tested out the code in https://github.com/WordPress/wordpress-develop/pull/353 with some recent core themes.
In general for single posts, things worked as expected, only the featured image at the top of the page excluded the loading="lazy"
attribute, inline content images all included it.
I noticed a potential issue however: when I set my home page to show recent blog post summaries, the themes included the featured image for each post: these images did lot include the lazy loading attribute. Similarly, visiting category archive pages showed a list of posts and these images were not lazy loaded - so I had a long page with images and none were lazy loaded - not ideal.
After testing I am inclined to agree with @azaozz and feel wp_get_attachment_image
should also default to lazy loading. Then we can clearly document for developers how they can opt out if they need a different behavior with the wp_lazy_loading_enabled
filter.
#9
@
4 years ago
Thanks for the feedback @azaozz and @adamsilverstein! I will update the PR for now as mentioned, so that wp_get_attachment_image
keeps lazy-loading on by default. The improved support for opting out (if opting out, the recommendation is to use null
or false
, but not 'eager'
) plus the extra test coverage is useful to be merged already, regardless of the final decision here.
I'll keep this ticket open so we can continue testing and discussing as needed.
This ticket was mentioned in Slack in #core-media by antpb. View the logs.
4 years ago
#12
@
4 years ago
- Keywords has-dev-note added; needs-dev-note removed
Dev note published: https://make.wordpress.org/core/2020/07/14/lazy-loading-images-in-5-5/
#13
@
4 years ago
- Resolution set to fixed
- Status changed from assigned to closed
I'm going to close this one out in favor of new, more specific bug report tickets. If there are still tasks required that I missed, feel free to reopen.
#14
in reply to:
↑ 3
@
4 years ago
Replying to flixos90:
Replying to joemcgill:
get_the_post_thumbnail()
is used extensively in places where the featured image is not in the viewport, e.g., on archive pages or in places where some UI is being creating using a secondary query, so we might want to be more explicit in that case that it only loads whenis_single()
andis_main_query()
are both true.
That's a fair point. I'd assume in the majority of cases the image would be somewhere near the top of the page, but it's true we cannot rely on that. I think it would make most sense to provide essential support for the lazy-loading and rely on themes to manage the specifics for these kinds of images, based on where they place them. I'll post a follow up comment with more details on the proposed solution.
I noticed that in three most recent core themes (at least) the first featured image is still getting loading=lazy
even though the image appears in the first viewport. So the themes aren't managing the specifics as they should be.
I think that whenever a featured image is being printed (via the_post_thumbnail()
) that the resulting image should not get loading=lazy
if the image being printed is:
- The featured image for the queried object on a singular template, or
- The featured image for the first post in the main query.
I put together a prototype of how I think this can be addressed (though it does require a bit of a action/filter dance): https://gist.github.com/westonruter/e8d5778843005b7e0d6ce4049b3ec29d
Thoughts?
get_the_post_thumbnail()
is used extensively in places where the featured image is not in the viewport, e.g., on archive pages or in places where some UI is being creating using a secondary query, so we might want to be more explicit in that case that it only loads whenis_single()
andis_main_query()
are both true.