#44427 closed enhancement (fixed)
Introduce lazy-loading API for media and other elements
Reported by: | mor10 | Owned by: | flixos90 |
---|---|---|---|
Milestone: | 5.5 | Priority: | normal |
Severity: | normal | Version: | 5.4 |
Component: | Media | Keywords: | has-patch has-unit-tests 2nd-opinion early needs-testing has-dev-note |
Focuses: | performance | Cc: |
Description
Synopsis
Lazy-loading of media and other elements (iframes, embeds, etc) is a performance best practice. Currently, various plugins including Jetpack, and themes including WP Rig ship lazy loading features to improve performance. These solutions rely on conditionally rewriting the HTML for images and other media as it is being loaded from core which is sub-optimal.
Proposal
Introduce a lazy-loading API in core which lazy-loads media and other elements out of the box based on various conditions, with an opt-out feature in the Customizer. This takes the onus of having extensive knowledge of performance optimization and how to add lazy-loading to your site off the shoulders of the end-user and gives developers control over how and when content is lazy-loaded.
The lazy-loading API should allow theme- and plugin developers detailed control over how lazy-loading works. As an example, the AMP plugin would disable core lazy-loading and replace it with its own custom method. Another example: A theme developer may choose to immediately load the featured image of the first post in an index or archive template while all other images are lazy-loaded.
Why this isn't plugin territory
Lazy-loading is currently considered plugin territory. I disagree for a couple of reasons:
- Lazy-loading is performance best-practice: https://developers.google.com/web/fundamentals/performance/lazy-loading-guidance/images-and-video/.
- WordPress should not put the onus on site-owners to have in-depth knowledge of performance best-practices and how to meet them.
- With no consistent API, plugins and themes may introduce different and conflicting solutions to the same problem which in turn causes unexpected behavior for the end-user.
- Lazy-loading introduced at plugin/theme level forces aggressive rewriting of content which would be more consistently performed at core.
Attachments (11)
Change History (138)
#2
follow-up:
↓ 3
@
6 years ago
This feels like a great win for performance, standards, carbon, etc. The SEO and performance nerd in me loves this.
Is there any reason why we can't/shouldn't implement this, verbatim, as per the Google documentation (using the event handlers
method, until such time as the intersection observer
method is universally supported)?
Given how minimal the JS size and processing overhead is, and given that we'll want lazy-loading to be default on (at least, in future/ongoing theme dev etc), I've no objections to the JS being autoloaded on all front-end requests (unless it's specifically disabled/de-enqueued).
I do think that there may be some nuance in implementation; some scenarios:
- Inline images, inserted in content; where we'll be adding a bunch of
data-
params to the code. We'll need any existing or new filtering/validation to anticipate and manage this (or, to transform the code via a filter onthe_content
, if we're concerned about code bloat or confusion in the content editor).
1a) Gutenberg blocks.
1b) images in non-webpage context / where js isn't available, like REST or RSS.
- Theme/template images which use
wp_get_attachment_image()
or similar to output image HTML. We'll need to hook into these with an additional parameter (we shouldn't make this a universal default/assumption, as many themes use 'hero images' in header areas where lazy-loading may produce an unwanted delay).
- Theme/template images which use
wp_get_attachment_image_srcset()
or similar to construct the output of theimg
/picture
etc tag. This poses a challenge.
The first and the second feel achievable; some settings, an extra parameter in the case of 2, and everything else can happen under the hood.
The third feels like the messiest for easy adoption; we're relying on a significant change to coding approaches, which @mor10 rightly wants to avoid. Thoughts appreciated!
#3
in reply to:
↑ 2
@
6 years ago
Is there any reason why we can't/shouldn't implement this, verbatim, as per the Google documentation (using the
event handlers
method, until such time as theintersection observer
method is universally supported)?
I like this idea, relying on a method that has been recommended by one of the expert sources in the field seems like a solid approach to me. We might even implement both ways in one script, and only execute the modern one if "IntersectionObserver" in window
. Otherwise use the event listener one.
Before we dive into implementation further, I think we need to discuss how this should integrate with themes.
- Given the need for some images to always load immediately and skip lazy-load, I suggest we only lazy load when the theme does
add_theme_support( 'lazy-load' )
. While this will result in adoption of lazy-loading not drastically increasing immediately, experience has proven that, over time, most theme features introduced were at a certain point commonly supported by most themes (think abouttitle-tag
for example). We just need to document well, make supporting the feature easy and encourage it. I think by making it opt-in, we're on the safe side. Also, some themes include this already, and there are plugins implementing that, so with opt-in we ensure that there are no conflicts for those sites. - I think we should only account for what we can reasonably account for.
<img>
tags in filterable content should be adjusted, as well as any calls towp_get_attachment_image()
. However, when people use other techniques (i.e. "hard-code" the<img>
tags themselves, what you @jonoaldersonwp mentioned too), we shouldn't automatically touch this, as it would require some output buffering, which we should avoid IMO. Documentation should rather highlight that this way of including images is not recommended andwp_get_attachment_image()
should be used instead. Something I'm not sure about is how to deal with background images as there's no standardized way of having that markup generated (also it's only markup if through an inlinestyle
attribute, otherwise in a CSS file, which makes this more tricky). - We need to figure out a way to easily allow theme developers to indicate that specific images must not be lazy-loaded. This could mean having a specific CSS class for this, or such thing as a
data-skip-lazy-load
attribute. Possibly, we could allow content creators to handle this as well, by putting a checkbox into the "Insert Media" modal somewhere, which says "Skip lazy-loading for this image?" - and if so, put the class or attribute into the generated tag.
#4
@
6 years ago
- Agreed on requiring explicit theme support.
- Agreed on only catching common/reasonable use-cases (e.g.,
<img>
tags andget_attachment_
derivatives) - Suggest we ignore background images (for now), as this adds a ton of additional complexity and overhead.
- Agree on
data-
attributes (and/or a class name) for skipping lazy load, and for a checkbox against the media entity.
So, specifics:
- We need to agree on a theme support hook. I propose
lazy-load-images
for some additional specificity, as we might also do something withlazy-load-{{other}}
in the future. - We need a checkbox against all media where the mime type is of an image format and the wording around that. E.g., "Disable lazy-loading for this image".
- We need to agree a
data-
attribute or class name for skipping lazy load. Agreed ondata-skip-lazy-load
. Whilst I'd prefer an attribute, my gut feeling is that might be a little foreign to most content editors and some theme developers; whereas a class (skip-lazy-load
) might be a more recognisable approach. No reason we can't support both. - We need to identify the
get_attachment_
functions which return image HTML (as opposed to attributes) and assess the impact of filtering the output of these. - Update relevant documentation.
- Build the thing.
#5
@
6 years ago
Here is a list of functions we identified, which outputs HTML for images and have an available filter:
get_avatar()
https://developer.wordpress.org/reference/functions/get_avatar/get_image_tag()
https://developer.wordpress.org/reference/functions/get_image_tag/get_the_post_thumbnail()
withpost_thumbnail_html
filter https://developer.wordpress.org/reference/functions/get_the_post_thumbnail/
get_the_post_thumbnail()
uses wp_get_attachment_image()
under the hood, which doesn't have a filter for the whole HTML returned (see https://core.trac.wordpress.org/ticket/27399), only for the attributes with wp_get_attachment_image_attributes
.
In the case of images added into the main content and in widgets like the image widget, I suppose the best approach would be to add the attributes directly to the generated HTML at that time, the same way than for the responsive attributes.
#6
@
6 years ago
Ah, yes, maybe we should have a data-enable-lazy-load
attribute, too, for cases where an image might otherwise have been passively excluded, but the theme/content author wants it to be explicitly included.
#8
follow-up:
↓ 9
@
6 years ago
Lazy loading is becoming a web standard, with implementation in Chrome just around the corner. This should be the model anything with lazy loading is based on in WordPress core:
https://addyosmani.com/blog/lazy-loading/
https://groups.google.com/a/chromium.org/forum/m/#!topic/blink-dev/jxiJvQc-gVg
This is non-destructive and backwards compatible. Shipping it in WP core will lay down the necessary cowpath for widespread browser support. I suggest we work with the team behind this feature proposal to make a best-practice implementation.
Some suggestions:
- all images
lazy
by default. - first featured image on singular view
eager
by default. - API for theme / plugin developers to control
loading
attribute on case-by-case basis. - optional: core to ship JS shim for backwards compat.
#10
@
6 years ago
Agreed, and it also simplifies the implementation as no additional Javascript would be required by default. If adding a shim, it should only support IntersectionObserver
now, as it's supported in all modern browsers now (Safari 12.2 was the last to adopt it).
This also removes the need to brainstorm on data attributes to use to allow/disable lazyload.
Adding support via add_theme_support( 'lazy-load-images' );
and add_theme_support( 'lazy-load-iframes' );
will allow themes and plugins developers to provide a specific value for any image/iframe handled.
Examples: adding a loading="eager"
value for the logo in the header for a theme or the first slide of a slider created with a slider plugin, while the rest of the slides have the loading="lazy"
value.
#11
@
5 years ago
- Focuses performance added
- Keywords needs-patch needs-unit-tests added
I have create an issue on the github repo for gutenberg for this.
I think we can get this in WP 5.3.
Developers can start playing with this attribute with a filter as simple as this.
add_filter( 'wp_get_attachment_image_attributes', function( array $attr ) {
$attr['loading'] = 'lazy';
return $attr;
});
#12
@
5 years ago
While I like this idea, I think it should be pointed out that we should not re-invent the wheel with a lazyload library. Not sure if that was anyone thought process, but seemed like it. One that I have seen google link to (forgot where) and as a link on pagespeed I think, is https://github.com/aFarkas/lazysizes.
They also have an IntersectionObserver
version at https://github.com/aFarkas/lazysizes/blob/gh-pages/src/lazysizes-intersection.js which uses no scroll timers. The code uses no jquery and also makes changes with requestAnimationFrame with a simplistic queue system for performance vs setTimeout. Among it is a folder with like 20 plugins, thus can be used for custom ones.
Thanks.
#13
@
5 years ago
Native lazy-loading is now shipping in the latest version of Chrome. This is a live feature on the web with graceful degradation built in. Implementing it in WP Core is non-destructive and straight forward. Time to move!
#15
@
5 years ago
In all these discussions of lazy loading I don't see anyone considering that lazy loading on a slow connection makes for an absolutely abysmal browsing experience. Has no one browsed the web on a slow connection before? If I'm loading a long-ish article with images on a slow connection, then I want those images to start loading immediately so that they're visible by the time that I get to them. If lazy loading is enabled, or starts loading images too late, then the user is forced to stop and wait for the image to load each time they reach one. It's the equivalent of these web video players that only buffer a small amount of the video. When I had a poor connection in the past there were sites whose videos I could not watch at all (ever) because it was impossible to let the video load. This is a similar issue. It saves the site owner bandwidth, but makes browsing/watching utterly miserable.
I'd also like somebody to actually articulate what the performance benefits for lazy loading actually are before anything is done. This ticket and the Google article go on about performance, performance, performance, but with no real data. Obviously without lazy loading the page isn't considered fully loaded until all the images are loaded, which means that a page with lazy loading would be considered "fully loaded" sooner, but is that meaningful? One could argue that by that metric a page with lazy loaded images isn't truly fully loaded until I scroll to the bottom to cause all the images to load. Does the browser loading images below the fold actually significantly affect time to interactive? Does that outweigh the user experience costs I've described? We're seeing browsers and libraries specifically designed to pre-load entire webpages in the background so that when the user clicks a link they perceive the next page to have loaded instantly, but we're simultaneously seeing efforts to prevent content on the same page from being pre-loaded, also in the name of performance. How does that make sense?
Google themselves say:
Today, Chrome already loads images at different priorities depending on where they're located with respect to the device viewport. Images below the viewport are loaded with a lower priority, but they're still fetched as soon as possible.
Is that not already sufficient for perceived performance? How does lazy loading improve performance over that?
So my argument is that while lazy loading might help with bandwidth and memory usage, which is not meaningless, it is a considerably poorer user experience for anyone that is not on a high speed connection. As such I believe that any application of lazy loading needs to carefully consider which images it's used for, and that this is highly dependent on context and not a decision that WordPress should be making bluntly for all sites like this. If even Google believed that lazy loading was preferable for all (or even most) contexts, they would have made it the default browser behaviour for loading images. So why should WordPress make this behaviour the default? I believe that at most WordPress should add a checkbox for enabling lazy loading to the image insert/edit modals.
Perhaps I've just had experience with poor implementations, but this just boils down to me really wanting to see more consideration of the user experience of lazy loading, as well as some articulation of what the performance benefits actually are if performance is the justification (and not bandwidth/memory usage).
#16
follow-up:
↓ 126
@
5 years ago
@JakePT The main purpose of performance enhancements is exactly what you are asking for: Creating better user experiences for people on slow connections or with slower / older / less fancy devices. Reducing the weight of a page load, optimizing the delivery of assets, all these things benefit people on slow connections and/or devices more than it benefits people on broadband with top-of-the-line devices.
Lazy loading was created specifically to address page load for people on slower connections because of large page sizes mainly caused by images appearing "below the fold" and out of view. There's ample research to show people rarely scroll all the way down a page, and loading images out of view is a waste of valuable resources which incurs cost and slowness for the end-user, especially on slow connections. By lazy loading images and other heavy assets, the user is delivered a significantly improved user experience while at the same time saving both time and money.
The specific spec from Google, which is now implemented in Chrome, is to use the loading
attribute to instruct the browser on how to lazy load images and iframes. By default, the browser treats all images and iframes as loading="auto"
meaning the browser automatically lazy loads assets based on its own formula. Developers can set each asset to either loading="lazy"
which "defers loading of the resource until it reaches a calculated distance from the viewport", or loading="eager"
which "loads the resource immediately, regardless of where it's located on the page."
My proposal is to build this functionality into WP core to give developers and users direct access to this functionality:
- Developers can tag individual images or image types with either
lazy
oreager
to control how they are loaded. A good example would be featured images being set toeager
because they typically appear at the top. - Users could be provided with a toggle to choose if they want individual images to load
lazy
oreager
, but this is quite advanced and puts the onus on the end-user to know what lazy loading means and how it impacts performance.
Two crucial aspects of the proposal by Google are:
- Unlike current implementations of lazy loading which use JavaScript and an IntersectionObserver or similar to load images when they appear within the viewport, the new spec loads the image when it is within a "calculated distance threshold" (https://web.dev/native-lazy-loading#load-in-distance-threshold) meaning the image is likely to be loaded before it hits the viewport, thus reducing the risk of the scenario you are worried about with images not appearing when they are supposed to.
- The proposal is closely related to the
intrinsicsize
attribute proposal (https://github.com/WICG/intrinsicsize-attribute) which is slowly rolling out and will make it easier for browsers to clear space for yet-to-be-loaded images and other assets.
This ticket was mentioned in Slack in #core-js by pierlo. View the logs.
5 years ago
This ticket was mentioned in Slack in #core-js by pierlo. View the logs.
5 years ago
#19
follow-up:
↓ 20
@
5 years ago
I've added lazy loading of images to theme screenshots, as @joyously suggested here.
I used the lazysizes library for implementing it, and added the native-loading
plugin to automatically transform img.lazyload
elements in browsers that support native lazy loading.
The media library page can also benefit from this.
#20
in reply to:
↑ 19
;
follow-up:
↓ 22
@
5 years ago
Replying to pierlo:
I've added lazy loading of images to theme screenshots, as @joyously suggested here.
I used the lazysizes library for implementing it, and added the
native-loading
plugin to automatically transformimg.lazyload
elements in browsers that support native lazy loading.
The media library page can also benefit from this.
Is there a reason for this approach, vs the native Chrome approach described above?
#21
@
5 years ago
On the topic of lazysizes/intersection observer - this is something that was asked of Alexander (the creator of lazysizes), who recently wrote up a fairly insightful comment on the GitHub issue that was raised requesting it: https://github.com/aFarkas/lazysizes/issues/263#issuecomment-518340398
As was mentioned above, if lazysizes is being considered as a possible solution, there's a "native" plugin for lazysizes that caters for the browser native implementation of lazyloading: https://github.com/aFarkas/lazysizes/tree/gh-pages/plugins/native-loading
#22
in reply to:
↑ 20
@
5 years ago
Replying to jonoaldersonwp:
Is there a reason for this approach, vs the native Chrome approach described above?
Native Chrome approach = Chrome only (https://caniuse.com/#search=loading - currently 0.15%)
lazysizes + native-loading plugin = multiple browsers (lazysizes for those that don't support the loading attribute, native for those that do)
#23
@
5 years ago
Makes sense. However, that changes the spec and the requirements, and introduces a lot of complexity.
Moving to a data-src
based approach has very different implications from an integration, standards, performance, accessibility and SEO perspective - even with the hybrid approach. It has many more moving parts than adding support for a new attribute. Handling this is not impossible, but it's much more nuanced.
I'd much rather we go launch with adding support for loading=lazy
, and not change anything else; and that we 'pave the cowpath' by encouraging other browser vendors to catch up.
#24
follow-ups:
↓ 26
↓ 28
@
5 years ago
Adding the new loading
spec is non-destructive and adds no new loads on sites. Adding a JS library adds significant load and complexity. The current CanIUse stats for loading
is based on a week-old release of Chrome. It will shoot up very fast. More importantly, WP adding loading
will increase the percentage of sites using the spec exponentially, effectively carving a cowpath for the browsers to follow. This is very much a question of what role WP wants to play on the web: A leader in pushing the web forward, or a stalwart falling back on old tech when new solutions exist.
From my perspective there are two options: Either we implement the loading
spec and move the web forward, or we do nothing. A JS solution runs counter to the idea of native lazy-loading and may end up delaying the implementation on the web significantly.
In my opinion.
#26
in reply to:
↑ 24
@
5 years ago
Replying to mor10:
Adding the new
loading
spec is non-destructive and adds no new loads on sites. Adding a JS library adds significant load and complexity. (…)
Totally agree. 😊
#27
@
5 years ago
Sorry, I should've clarified!!! I don't think lazysizes should be included as part of core. There's too much configurability/complexity to it, I agree! My points were simply in relation to the other comments/questions around it :)
If core catered for lazysizes... it'd also have to cater for every configurable option/plugin - that would be quite a burden and is definitely still theme/plugin territory?
It would definitely be good to see the native lazyloading spec/attribute get a real push!!
#28
in reply to:
↑ 24
@
5 years ago
Replying to mor10:
Adding the new
loading
spec is non-destructive and adds no new loads on sites. Adding a JS library adds significant load and complexity. (...).
Agreed. So is it the consensus that the loading
attribute should be added to 'pave the cowpath'?
#30
@
5 years ago
- Keywords has-patch added; needs-patch removed
As a talking point, I have uploaded a first patch to this ticket. This uses a new feature flag. To enable lazy loading images, simply add theme support for lazy-loading-images
. This is something I discussed with @joemcgill .
The function is designed to be easily to lazy loading to iframes later as well.
I don't personally believe we need a javascript shim here for backwards compability. We don't do this with srcsets correctly.
#31
@
5 years ago
Thanks for the patch @spacedmonkey
It seems that lazy loading is currently used in plugins in various ways.
I had a peek into the plugins directory:
https://wpdirectory.net/search/01DJFY0YHPHJ76JWB6EW5SXCNK
that contains image tags like:
<img data-src="... <img data-lazy-src="... <img data-lazyloaded="1"... <img data-flickity-lazyload="... <img data-gt-lazy-src="... <img data-lazy="... <img class="lazyload" ... <img class="lazy lazy-hidden" data-lazy-type="iframe" data-lazy-src="... <img class="supsystic-lazy" data-original="... <img class="rocket-lazyload-rocket-logo" ... <img class="lazy_load_image" ... <img class="owl-lazy" data-src="... <img class="ready-lazy" data-original="... <img class="lazy" data-original="... <img class="lazyOwl" ... <img class="listing lazy" data-original="... <img class="shopapp-lazy" data-original="... <img class="responsively-lazy" ... <img class="fullWidthfileImg lazy" data-original="... <img data-no-lazy="1" src= <img class="skip-lazy" src <img class="skip-lazy gmw-image"
Not all af these might be part of the content, but I wonder how mixing it with loading="lazy"
will affect the already lazy loaded content images.
If they can't mix, then more aggressive checks come to mind to avoid it. Rather than checking only for the ' loading='
as in 44427.2.diff, one could avoid replacements on images that contain data
attributes for original, src, lazy, lazy-src, source, ... etc. But I think that might still miss out lot of other cases. (filterable?)
Making the flag available to themes, rather than having it ON by default, sounds safer, at least to start with.
#32
@
5 years ago
They can mix fine. All of those approaches are just hooks for JS systems which watch for element visibility/similar. The native loading=lazy
won't interfere with that (though it may be rendered redundant in some cases).
#33
@
5 years ago
@jonoaldersonwp thanks for clarifying that. I guess it would be like running both the native way and a JS fallback, yeah that was my worry. I will just try this out when I can :-)
I assume there might also be content cases like:
<img data-no-lazy="1" <img class="skip-lazy" ...
i.e. if the user has explicitly set the content images to not lazy load.
I think those cases should be respected, if possible, so that they would not be lazy loaded regardless.
So in addition to defining a suitable flag to avoid this, the existing flags out there, should be respected, if possible.
#34
@
5 years ago
I've never seen an example like that in the wild. In every case (and library/approach, etc) I've ever seen, images are either 'vanilla', or have classes/attributes/properties to enable lazy loading. I've never seen an exclusion-based approach.
#35
@
5 years ago
JetPack supports exclusion of lazy loading images with the skip-lazy
class or custom classes:
You can instruct Lazy Images to skip any image using the
skip-lazy
css class or any given class of your choice by using thejetpack_lazy_images_blacklisted_classes
filter.
https://jetpack.com/support/lazy-images/
https://developer.jetpack.com/module/lazy-images/
Another example of exclusion with the data-no-lazy="1"
attribute:
https://docs.wp-rocket.me/article/15-disabling-lazy-load-on-specific-images
#36
@
5 years ago
Gah. Trust those two to have frustrating anti-patterns.
Ok, yeah, we should sniff for those!
#37
@
5 years ago
These examples of custom- and anti-patterns are precisely why we need this implemented in core asap: Lazy-loading is becoming more and more well-known by site owners, and they are turning to plugins to get it done. Rolling out a cohesive method with a proper developer interface in core allows plugin developers to upgrade (or sunset) their solutions in a graceful way.
I am wary of any attempt at modifying the spec to conform to plugin behavior. A better solution from my perspective would be to notify all plugins which offer lazy-loading functionality of the upcoming change to core and give them time to test and update their solutions to fit.
#38
@
5 years ago
Dropping in a hello from the Chrome Speed team 👋
Lazy-loading is becoming more and more well-known by site owners, and they are turning to plugins to get it done.
Strongly agree. I'm very supportive of core getting support for native lazy-loading in a non-destructive manner.
It seems that lazy loading is currently used in plugins in various ways.
In my research, a majority of existing JavaScript solutions for lazy-loading rely on users switching from setting an image src
to data-src
(or similar) in order to prevent browsers from eagerly loading images.
If we have an image <img class="some-custom-loader" data-src="foo.jpg">
, a browser supporting native lazy-loading will not do anything special if an author adds loading=lazy
. Until the library/plugin/server modifies data-src -> src, we don't consider the image as having a valid reference.
The ideal change I would love to see in lazy-load plugins is deferring to native lazy-loading where supported and applying their fallback where it is not. I documented a few patterns for this in https://addyosmani.com/blog/lazy-loading/, but I'm certain something more ergonomic is possible. tl;dr: check for native image lazy-loading support and conditionally load a fallback (like lazysizes) when it isn't supported.
While the above model might work well for a plugin, I think a simpler model of core just supporting loading=lazy
as a progressive enhancement is a good path forward.
Anecdotally: Over 17K origins are currently using loading=lazy
(based on our telemetry) and large sites like the BBC are already starting to see speed metrics improvements as a result of adopting it https://twitter.com/addyosmani/status/1160939251426418689
#39
follow-up:
↓ 88
@
5 years ago
I think the patch by @spacedmonkey is a good first step in the right direction.
@jonoaldersonwp
I've never seen an exclusion-based approach.
[...]
Trust those two to have frustrating anti-patterns.
I think for the WordPress ecosystem a blacklist approach is more beneficial than an whitelist approach. If we relied on whitelisting images to lazy-load, we would probably never get to a wide adoption, because most users wouldn't know about the possibility entirely. With blacklisting, we lazy-load as much as possible by default, and if someone wants to exclude certain media from that, they can do so without negatively affecting the adoption of lazy-loading in WordPress. From my observations, I'd agree that one of the most common exclusion mechanisms appears to be a class skip-lazy
. We should decide on what mechanism we'd want to support for exclusion for a core implementation.
I'd also like to confirm @addyosmani's point about interoperability: Sites that currently use a JavaScript-based approach for lazy-loading won't be affected, as for these solutions there most likely won't be a src
attribute on the images.
I have one major concern about the currently proposed route for core, which is relying on theme support. Lazy-loading images is a general site feature that ties in with any functionality on the site and is not specific to themes. Furthermore, requiring theme support would leave the feature largely unused, since basically every theme in the world would need to be updated to add_theme_support( 'lazy-loading-images' )
first. If we want to easily allow enabling/disabling lazy-loading, I'd much rather prefer a user-centric, sitewide approach. That could either be an option to enable/disable it (e.g. a core-shipped Customizer control), or a filter.
#40
@
5 years ago
What @flixos90 said: lazy loading should be on by default, and theme and plugin developers can choose to modify it for specific images.
the onus can not be on the end-user to know to turn on lazy-loading or to look for a specific theme that supports it. Asking theme developers to incorporate the feature in a consistent way is also a path to inconsistent application and slow adoption.
#41
@
5 years ago
I agree with @flixos90 and @mor10 that lazy loading should be enabled by default.
By WordPress adapting what Chrome has added, we will move the web forward and improve website speed. This will also encourage other browsers to use the same standard.
#43
@
5 years ago
- Milestone changed from Awaiting Review to 5.4
- Owner set to flixos90
- Status changed from new to assigned
44427.3.diff expands on the previous patches:
- It also lazy-loads iframes with the
loading
attribute, in addition to images. - It relies on a filter
wp_lazy_load_content_media
instead of theme support, since lazy-loading is not really a theme-related feature, based on conversations at WCUS contributor day. - It lazy-loads media by default. The filter can be used to opt-out for specific instances, or also globally in edge cases where that would be needed.
- It simplifies the code a bit, using
preg_replace_callback()
instead of a foreach loop with individual replacements.
cc @spacedmonkey @adamsilverstein
#44
@
5 years ago
In addition to:
<?php add_filter( 'the_content', 'wp_lazy_load_content_media' );
What about also adding it to widget_text_content
, the_excerpt
, the_post_thumbnail_caption
, comment_text
among some others perhaps?
Also, I believe wp_lazy_load_content_media
should be run after convert_smilies
which runs at priority 20 (at least for the_content
).
#45
follow-up:
↓ 50
@
5 years ago
@westonruter
What about also adding it to
widget_text_content
,the_excerpt
,the_post_thumbnail_caption
,comment_text
among some others perhaps?
Some of those might make sense, but probably not all. I'd lean towards doing it for widget_text_content
possibly, but the others typically don't include images I'd say?
Also, I believe
wp_lazy_load_content_media
should be run afterconvert_smilies
which runs at priority 20 (at least forthe_content
).
Can you clarify how these two filter functions are related? The existing function wp_make_content_images_responsive()
which also makes media element replacements also runs at the default priority 10.
#46
@
5 years ago
- Keywords has-unit-tests added; needs-unit-tests removed
44427.4.diff adds unit test coverage.
#47
@
5 years ago
Might make sense to add the filter to render_block
in addition to widget_text_content
.
Also, the regex should be case insensitive
It also lazy-loads iframes with the loading attribute, in addition to images.
Would be nice to add this attribute to get_post_embed_html()
and whitelist the attribute accordingly in the oEmbed handler.
#48
@
5 years ago
@swissspidy
Might make sense to add the filter to
render_block
in addition towidget_text_content
.
I'm not sure about render_block
, since that content would usually be covered already by the_content
right?
Would be nice to add this attribute to
get_post_embed_html()
and whitelist the attribute accordingly in the oEmbed handler.
+1
#49
@
5 years ago
I'm not sure about
render_block
, since that content would usually be covered already bythe_content
right?
True, unless the block is displayed in a widget area or block area.
#50
in reply to:
↑ 45
@
5 years ago
Replying to flixos90:
Also, I believe
wp_lazy_load_content_media
should be run afterconvert_smilies
which runs at priority 20 (at least forthe_content
).
Can you clarify how these two filter functions are related? The existing function
wp_make_content_images_responsive()
which also makes media element replacements also runs at the default priority 10.
If convert_smilies()
causes img
smiles to be output, then wp_lazy_load_content_media()
should run after it so that they also get loading=lazy
. Either this, or the translate_smiley()
function could itself include loading=lazy
when constructing the img
.
#51
follow-up:
↓ 52
@
5 years ago
In 44427.5.diff made a couple of key changes.
- Add lazy loading attr on smiley images.
- Add filter the
the_excerpt
- Add filter the
comment_text
I looked at ending the filter to widget_text_content
, but I am sure of how much of a benefit this is.
#52
in reply to:
↑ 51
@
5 years ago
Replying to spacedmonkey:
I looked at ending the filter to
widget_text_content
, but I am sure of how much of a benefit this is.
The Text widget has TinyMCE in it which allows adding images, so it was ensure those images also get lazy loaded. This filter would apply that change to the widget.
#53
@
5 years ago
I made some key changes in 44427.6.diff
- Added filter to
widget_text_content
( Props @westonruter ) - Refactored logic so that all html is change by adding the
wp_lazy_load_content_media
filter. - Added
$content
towp_lazy_load_content_media
filter for better context. - Added
smilies_html
filter. - Added
wp_get_attachment_image
filter.
I know it more expensive ( from a compute standard point ) to run the regex in all these places, how, I think the added control for developers to be able to unhook these filters, is a great trade off.
Adding a filter to wp_get_attachment_image
also fixes #27399.
#54
@
5 years ago
Another filter we may want to think about implement lazying loading in is the header image that uses get_header_image_tag
. In 99% of case, this image will be loaded first on the page, making adding the filter a little pointless. But maybe worth it for the sack of completeness.
I also reviewed the get_custom_logo
function, but this seems to use wp_get_attachment_image
, which is already filtered.
#55
@
5 years ago
@spacedmonkey I left a review for you here: https://github.com/WordPress/wordpress-develop/pull/123#pullrequestreview-328008233
#56
@
5 years ago
I have left my responses on the review - https://github.com/WordPress/wordpress-develop/pull/123
Thanks for taking the time to review Weston
#57
@
5 years ago
@westonruter
Thanks for putting this PR together from the patches.
@spacedmonkey
Another filter we may want to think about implement lazying loading in is the header image that uses
get_header_image_tag
. In 99% of case, this image will be loaded first on the page, making adding the filter a little pointless. But maybe worth it for the sack of completeness.
I think we can omit that one for the reason you outlined.
I also left a review on https://github.com/WordPress/wordpress-develop/pull/123#pullrequestreview-328275558. I think introducing general filters here is out of scope, it doesn't directly relate to lazy-loading, especially since what the filters are used for could be easily accomplished without them (see 44427.5.diff).
I much prefer 44427.5.diff over 44427.6.diff because it's simpler and more focused on the issue. I think we should simply add the filter for widget_text_content
to 44427.5.diff, then it should be good.
#58
@
5 years ago
@flixos90 The reason for the change so that everything uses filters, as it not only does it make the code much cleaner, but it also gives developers much more control to unhook this behavior. Yes there was wp_lazy_load_content_media
filter, but this had no context to it. What if you wanted to unhook it from one place but not another. There is also more context to the wp_lazy_load_content_media
as this now passes the $content
as a param. Something that was not possible in @flixos90 's patch.
#59
@
5 years ago
@spacedmonkey
The reason for the change so that everything uses filters, as it not only does it make the code much cleaner, but it also gives developers much more control to unhook this behavior.
You can easily unhook wp_lazy_load_content_media()
from individual filters. Also I'd argue "context" from what PHP can give us here is largely irrelevant - context for lazy-loading media is much more dependent on browser viewport which is information we don't have here anyway.
I furthermore think introducing generic new filters for something that simple doesn't make the code cleaner, and it even has a performance impact: wp_lazy_load_content_media()
runs a regex that adds loading
attributes to all applicable tags in a string. Running it on a single image tag for example is overkill. Just because we can reuse the function doesn't mean we should.
Let's take a step back and focus on what we're trying to accomplish here: The goal is to add support for the loading
attribute to core. Eventual improvements and more flexibility can be added on demand in the future as necessary.
#60
@
5 years ago
Let's take a step back and focus on what we're trying to accomplish here: The goal is to add support for the loading attribute to core. Eventual improvements and more flexibility can be added on demand in the future as necessary.
This needs to be build to allow for developers to easily unhook or 'turn off' this feature. This is not a future requirement, is something needed at launch. There is good reason you may wish to turn off selectively lazy load part tags. Making sure a video / tracking pixel are just two examples. The current patch was all or nothing, which seems a little extreme when we can give users options.
This ticket was mentioned in Slack in #core-media by mike. View the logs.
5 years ago
#62
@
5 years ago
44427.7.diff is based on 44427.5.diff:
- It limits default support status to
img
tags only and modifying the filter so that it can be used to whitelist specific tags to be supported (e.g. to also includeiframe
tags). I think only supportingimg
tags initially by default is a safer bet, and also way more important because of how much they're used - we can revisitiframe
support in the future, for now it's possible via filter. - It consolidates this logic into a function
wp_get_lazy_load_tags()
so that the filter does not need to be called in multiple locations. - It includes more solid logic to add
loading="lazy"
attributes to content strings, based on @westonruter's feedback in https://github.com/WordPress/wordpress-develop/pull/123. - For specific image functions, the attribute is added in these functions because running the catch-all
wp_lazy_load_content_media()
function would be overly expensive for such a case where we know the string is just a single tag.
This ticket was mentioned in Slack in #core-media by flixos90. View the logs.
5 years ago
#64
@
5 years ago
Reviewing 44427.7.diff
- Agree iframes should be removed, as they are problematic.
foreach ( array( 'the_content', 'the_excerpt', 'comment_text', 'widget_text_content' ) as $filter ) {
add_filter( $filter, 'wp_lazy_load_content_media' );
}
- These lines above. I think the priority should be changed to 8. This means it fire way before most plugins. It also means that added before blocks and shortcodes are added. This stops any image tags that are added in via a third party block/shortcode from getting the lazy loading attribute. There maybe good reason a develop might want a image to not lazy loading, like if they are using a tracking pixel in a shortcode (I have done myself so I know that happens).
- Should we disable lazy loading image if the theme has not
defined add_theme_support for html5? This would stop older theme written in XHTML from becoming invalid markup. A simple workout like this might work.
add_action( 'template_redirect', 'wp_remove_lazy_tags', 5 ); function wp_remove_lazy_tags(){ if( false === get_theme_support( 'html5' ) ) { add_filter( 'wp_lazy_load_content_media', '__return_empty_array'); } }
- I think we need more unit tests, especially ones with malforms img tags, like attributes on new lines, data attributes and the like.
Generally I think we are nearly there.
This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.
5 years ago
#66
@
5 years ago
If I understand properly, the recent patches have moved from an approach of having this be something that a theme supports and opts into, to a default inclusion of the attribute set to lazy
, for both existing and new images. Is this correct?
If so, could you please walk me through the risks/safety of changing that expected behavior for existing images and themes?
In particular -- I'm wondering why doesn't Chrome do this already, if it's safe, and what makes it different for WordPress, here.
#67
follow-up:
↓ 75
@
5 years ago
If I understand properly, the recent patches have moved from an approach of having this be something that a theme supports and opts into, to a default inclusion of the attribute set to lazy, for both existing and new images. Is this correct?
That is correct @mikeschroder. It is automatically opts you in.
Lazy loading images / iframes, is an opt in feature in chrome. There many use cases that require an image or iframe to be loaded on page load. A good example of which would be a tracking pixel. This image is not loaded, then a page view is not tracked. So the browser can not and should not force this behavior.
In the WordPress use case, image tag generated by PHP seems to be reasonable to make lazy loading. It is the some reasoning behind srcset, to increase loading performance and make user experience better.
#68
@
5 years ago
Change the priority of the 'the_content', 'the_excerpt', 'comment_text', 'widget_text_content'
filters. This means that less likely conflicts with plugins that add image tags to these filters.
#69
follow-up:
↓ 70
@
5 years ago
- Keywords 2nd-opinion added
This is an attempt to clarify couple of questions and a bit of a tl;dr (as the method of lazy loading has changed from needing js to adding an HTML attribute). Thinking this is a really good enhancement and the new loading
attribute should be added by default to all images. Also, perhaps it would be good to consider the importance
and decoding
attributes.
The more important question (imho): Is this an essential enhancement that would require "backporting" to all content ever published in WordPress?
- If yes, what are the chances the browsers would start lazy-loading images by default? Seems like that's the best way to implement it. Reading @addyosmani's excellent "introductory" post at https://addyosmani.com/blog/lazy-loading/ this seems quite possible?
- If no, does it warrant adding yet another runtime filter on all content outputted from WordPress that "parses" HTML with regex? This is slow, may increase page load time/use of server resources on the front-end quite a bit, and prone to edge-case errors/failures. Would it be better to add the attributes only in new content, for example in the image and gallery blocks, etc.
Another question is: Are there any downsides in automatically adding the loading
attribute to all img tags (either in new content or by regex at runtime)? Looking at the img
tag specs: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/img there are several (newer) attributes that may need considering. Most importantly the interactions between decoding
, importance
, and loading
. What happens when an image has importance="high"
and/or decoding="sync"
and WP adds loading="lazy"
? Also, should we be considering adding any of these attributes together?
Also have few remarks about 44427.8.diff. Thinking that, perhaps, some things can be done a bit better there but will review it separately after the above two questions are cleared.
#70
in reply to:
↑ 69
;
follow-up:
↓ 71
@
5 years ago
Replying to azaozz:
Thanks for the detailed feedback and thoughts!
The more important question (imho): Is this an essential enhancement that would require "backporting" to all content ever published in WordPress?
- If yes, what are the chances the browsers would start lazy-loading images by default? Seems like that's the best way to implement it. Reading @addyosmani's excellent "introductory" post at https://addyosmani.com/blog/lazy-loading/ this seems quite possible?
- If no, does it warrant adding yet another runtime filter on all content outputted from WordPress that "parses" HTML with regex? This is slow, may increase page load time/use of server resources on the front-end quite a bit, and prone to edge-case errors/failures. Would it be better to add the attributes only in new content, for example in the image and gallery blocks, etc.
I think this should be back-ported to all content because only then it will have a significant impact on performance of WordPress websites. We surely need to get it right, but I think only adding this to new content (e.g. via expanding the core/image
Gutenberg block type) would leave most WordPress sites without major improvements, leaving that chance unused. Furthermore doing it on the fly makes this more maintainable, as we'd be easily able to make further improvements in the future. In terms of the cost, that is certainly a consideration. However, I think the impact of one preg_replace_callback()
per post is rather low on a relative scale - the logic for adding loading
attributes is much cheaper than e.g. the one for making images responsive, it includes much less logic and no potential database call. Maybe we can optimize the patch further though, particularly the additional preg_match()
that runs for every match/image.
Regarding browsers lazy-loading images by default, I'm personally not aware of any such plans. Was there anything particular in the blog post that led you to this thought? In that regard, I'd be curious to learn more about the behavior of auto
- is there any chance that auto
today means eager
and at some point in the future would automatically become lazy
? cc @addyosmani
Another question is: Are there any downsides in automatically adding the
loading
attribute to all img tags (either in new content or by regex at runtime)?
I don't think there are. The loading
attribute is a progressive enhancement, browsers that don't support it will ignore it. Also, the browser implementation is rather conservative in my experience, mostly because users not seeing an image yet when it has just entered the viewport should be prevented. What is great here over pretty much any JS implementation is that e.g. Chrome fetches the initial 2KB of each image initially still so that the size of the image is known immediately, thus preventing later content shifting once the image has been actually loaded.
Looking at the
img
tag specs: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/img there are several (newer) attributes that may need considering. Most importantly the interactions betweendecoding
,importance
, andloading
. What happens when an image hasimportance="high"
and/ordecoding="sync"
and WP addsloading="lazy"
? Also, should we be considering adding any of these attributes together?
That's a good question. I haven't been able to dig up a resource on the combination of these three, but we should sort that out before moving this forward. I don't think we should expand this ticket further to cover adding these attributes too, but whether their combination can have unintended effects is crucial to know.
Also have few remarks about 44427.8.diff. Thinking that, perhaps, some things can be done a bit better there but will review it separately after the above two questions are cleared.
Sounds good, let's get back to the specifics of patches once we have identified the overall path forward.
#71
in reply to:
↑ 70
@
5 years ago
Replying to flixos90:
Thanks for the explanation and clarification! :)
I think this should be back-ported to all content because only then it will have a significant impact on performance of WordPress websites.
Right, this will have the highest/best impact. Lets do it.
I think the impact of one
preg_replace_callback()
per post is rather low on a relative scale - the logic for addingloading
attributes is much cheaper than e.g. the one for making images responsive, it includes much less logic and no potential database call. Maybe we can optimize the patch further though, particularly the additionalpreg_match()
that runs for every match/image.
Yes, thinking we can try to optimize the exact method used a bit more. Also perhaps not parse all <img>
tags twice with regex. Maybe the current functionality can be extended to add all needed HTML attributes at the same time/with one preg_replace_callback()
. Lets look a bit more there when reviewing the actual patch.
Regarding browsers lazy-loading images by default, I'm personally not aware of any such plans. Was there anything particular in the blog post that led you to this thought?
No, nothing "direct" in the post. But it explains it very well, including the "side effects". When thinking more about it, seems the best place for this enhancement is in the browsers, i.e. change the default from eager
to lazy
as you mention :)
Of course there is the caveat when the browser "doesn't know" the size of the image before starting to download it. As mentioned Chrome fetches the initial 2KB of each image file to try to get the (intrinsic) size. Generally this is "not ideal" as establishing another http(s) connection takes time and resources (there are usually 7 "back and forth" between the client and the server in an https handshake). I'm not 100% sure if that happens for all under-the-fold images or only when there's no width
and height
attributes in the img tag.
Perhaps this is something that can be considered when implementing it in WP? Would it make sense to make it conditional on having width
and height
and not having importance
and decoding
? Or should we ignore the presence of width
and height
and assume the browser will always be able to get/guess the image size and the webpage content will never "jump down" when the user scrolls and images are loaded. Thinking it would be great to try to "dig up" some more info about the "interactions" between these img tag attributes.
I don't think we should expand this ticket further to cover adding these attributes too (
importance
anddecoding
), but whether their combination can have unintended effects is crucial to know.
Yes, these two attribute seem very specific anyway, don't think we can auto-add them even if we wanted to, and seems they shouldn't be removed when present.
#72
@
5 years ago
I think that discussing importance
and decoding
is getting us a little off track here. If those attr are going to be added, it should be after this and use the existing regex for performance reasons.
IMO, I believe that browser will never automatically lazy load images. You never know when a image HAS to load. For example, tracking pixels or an important logo. The way that this feature implemented, so that developers have to opt in, so there is no breakage. Adding this attribute feels pretty safe here.
Would it make sense to make it conditional on having width and height and not having importance and decoding
Lazy loading should be added to all image, as height
and width
attributes are many times an incorrect and not representative of the size of the image.
This ticket was mentioned in Slack in #core-editor by spacedmonkey. View the logs.
5 years ago
This ticket was mentioned in Slack in #core by azaozz. View the logs.
5 years ago
#75
in reply to:
↑ 67
;
follow-up:
↓ 76
@
5 years ago
Replying to spacedmonkey:
Lazy loading images / iframes, is an opt in feature in chrome. There many use cases that require an image or iframe to be loaded on page load. A good example of which would be a tracking pixel. This image is not loaded, then a page view is not tracked. So the browser can not and should not force this behavior.
In the WordPress use case, image tag generated by PHP seems to be reasonable to make lazy loading. It is the some reasoning behind srcset, to increase loading performance and make user experience better.
Thanks for the reply! While both lazy loading and srcset are both performance-centered features for images, I think they're a bit different.
Users already had the expectation that those sizes were generated for the images' use with respimg, but it's seems like a pretty big behavioural change for images not to be loaded automatically at all.
IMO, I believe that browser will never automatically lazy load images. You never know when a image HAS to load. For example, tracking pixels or an important logo.
I'm concerned about this, but from the WordPress side of things. How can WordPress know what a user intended -- in particular with previous images/themes that weren't aware of this feature? I like the idea if we can do it safely -- just want to be sure that I understand what makes things different here.
A quick note that if it's decided to go ahead with this, I think it would benefit from a public proposal so that we can gather feedback from the wider community about potential side effects/edge cases.
#76
in reply to:
↑ 75
;
follow-ups:
↓ 78
↓ 79
@
5 years ago
Replying to mikeschroder:
I'm concerned about this, but from the WordPress side of things. How can WordPress know what a user intended -- in particular with previous images/themes that weren't aware of this feature? I like the idea if we can do it safely -- just want to be sure that I understand what makes things different here.
A quick note that if it's decided to go ahead with this, I think it would benefit from a public proposal so that we can gather feedback from the wider community about potential side effects/edge cases.
What if we restrict adding adding loading=lazy
by default to just images that are being loaded from the install's uploads
directory? This will avoid scenarios where we accidentally lazy-load images that are being used for tracking purposes, which would generally be pointing to some external domain or to some REST API endpoint.
#77
@
5 years ago
I think that discussing importance and decoding is getting us a little off track here. If those attr are going to be added, it should be after this and use the existing regex for performance reasons.
Frankly don't think discussing importance
and decoding
is off track here. It's not about adding them, it's how would their presence affect (or would it affect) adding loading="lazy"
. Reading the docs, they seem mutually exclusive. Seems WP should not be adding loading="lazy"
when there is importance="high"
and/or decoding="sync"
as both of them imply that the image has to be loaded "eagerly".
On the other hand it may be "OK" to add all of these attributes and the browser would prioritize them?
IMO, I believe that browser will never automatically lazy load images. You never know when a image HAS to load. For example, tracking pixels or an important logo.
I share @mikeschroder's concerns here. If the browsers would "never" lazy-load images by default, should WordPress be doing it? Seems that figuring out the "interactions" between importance
, decoding
, and loading
would alleviate some of these concerns.
Also, don't think this would be a problem for "tracking pixels" as the browsers fetch the first 2KB of the image. This does the "tracking" request. Also racking pixels are usually smaller than 2KB anyway.
I think it would benefit from a public proposal so that we can gather feedback from the wider community about potential side effects/edge cases.
Thinking this is an excellent idea! Perhaps a "request for feedback" post on https://make.wordpress.org/core/ with (concise) explanation of the benefits and concerns? This will also serve as an early dev. note about what may be coming in WP 5.4 :)
At the same time thinking that shouldn't slow down development of the patch. As far as I see, the "worst case" scenario would be that we release this as a feature plugin and (try to) solicit wide testing and feedback in an attempt to find all edge cases. Planning to look at 44427.8.diff later today or tomorrow and see if it can be optimized a bit.
#78
in reply to:
↑ 76
@
5 years ago
Replying to westonruter:
What if we restrict adding adding
loading=lazy
by default to just images that are being loaded from the install'suploads
directory?
Ha, we posted a replay at almost the same time :)
Yes, thinking this would be a good way to prevent eventual edge cases. If we end up making a feature plugin, perhaps this should be an option in it?
On the other hand it seems that lazy-loading is "safe" in the great majority of the cases. Having a way to opt-out, perhaps by adding importance="high"
, seems like a good way to handle it?
#79
in reply to:
↑ 76
;
follow-ups:
↓ 80
↓ 81
@
5 years ago
Replying to westonruter:
What if we restrict adding adding
loading=lazy
by default to just images that are being loaded from the install'suploads
directory? This will avoid scenarios where we accidentally lazy-load images that are being used for tracking purposes, which would generally be pointing to some external domain or to some REST API endpoint.
My understanding is that adding loading=lazy
to all images can be just as problematic, or potentially more so, than not using the attribute. Images within the view port ought not use the attribute while images below the fold ought to.
A Google performance article on lazy loading discusses some of the risks with lazy loading, although it's a little out of date and focuses on JavaScript technique I belive the same risks exist with the native techniques.
Additionally, and this is opinion rather than science based, in some cases the images are the primary content (attachment pages, posts that only contain a gallery/photo) so I'd argue these should be loaded normally.
Due to the high usage of WordPress, adding the attribute by default in certain cases needs to be considered carefully as there is a risk the cowpath (to use @mor10's term above) set defeats the intended purpose of the attribute.
#80
in reply to:
↑ 79
@
5 years ago
Replying to peterwilsoncc:
Replying to westonruter:
My understanding is that addingloading=lazy
to all images can be just as problematic, or potentially more so, than not using the attribute. Images within the view port ought not use the attribute while images below the fold ought to.
This shouldn't apply because loading=lazy
only affects behavior of images that are outside the viewport.
#81
in reply to:
↑ 79
;
follow-up:
↓ 82
@
5 years ago
Replying to peterwilsoncc:
My understanding is that adding
loading=lazy
to all images can be just as problematic, or potentially more so, than not using the attribute. Images within the view port ought not use the attribute while images below the fold ought to.
A Google performance article on lazy loading discusses some of the risks with lazy loading, although it's a little out of date and focuses on JavaScript technique I belive the same risks exist with the native techniques.
As @westonruter points out my understanding is that using the (new) img tag attribute "removes" these concerns. It lets the browser decide when to lazy-load, it does not "disable" the img tags like the js methods.
Form @addyosmani's introductory post:
The loading attribute allows a browser to defer loading offscreen images and iframes until users scroll near them. loading supports three values:
-- lazy: is a good candidate for lazy loading.
-- eager: is not a good candidate for lazy loading. Load right away.
-- auto: browser will determine whether or not to lazily load.
(Just to clarify: seems auto
currently is the same as eager
).
Additionally, and this is opinion rather than science based, in some cases the images are the primary content (attachment pages, posts that only contain a gallery/photo) so I'd argue these should be loaded normally.
Right, but in (almost?) all of these cases these "high-priority" images are also visible in the initial viewport. As I mentioned above, it makes sense to "examine" the interactions between importance, decoding, and loading. Seems any high-priority images should have importance="high"
set, whether that affects lazy-loading or not.
@peterwilsoncc what do you think about the idea to make this into a (featured) plugin and attempt to promote more user testing? I'm pretty sure there will be some large/busy sites that may want to participate in such tests :)
#82
in reply to:
↑ 81
@
5 years ago
Replying to azaozz:
@peterwilsoncc what do you think about the idea to make this into a (featured) plugin and attempt to promote more user testing? I'm pretty sure there will be some large/busy sites that may want to participate in such tests :)
I like the idea of a feature plugin and am happy to be involved there. It would also allow for some perf testing to be added via CI to gain some data rather than hypothesis ;)
#83
@
5 years ago
There is a feature plugin that was written by the google team called Native Lazyload.
Can we just make this an opt in feature using a theme supports? This fixes a lot of issues, as a developer know better.
#84
follow-up:
↓ 85
@
5 years ago
I like the suggestions by @mikeschroder @peterwilsoncc @azaozz to:
- prepare a proposal post for the make blog
- create a feature plugin for easy testing
I'll kickstart these two things and will invite all of you for collaboration and review.
With both of this together, we'll be able to get more feedback on the interoperability with people's sites, whether this causes any unexpected side effects.
@spacedmonkey Our Lazyload plugin encompasses a bit more than the core-envisioned solution (as in the patches here), so that IMO isn't suitable as feature plugin. I'll transfer the latest patch 44427.8.diff into a feature plugin as is.
Regarding relying on theme support, I'd advocate against this. It would potentially allow us to get it merged more quickly, but I feel like we shouldn't do this because
- either it becomes one of these conventions that every theme adds support to, which is pointless, and would then result in it being de-facto always on anyway (which defeats the purpose of theme support)
- or it doesn't become one of these conventions, which will result in probably 5% of themes adding support which largely limits the positive performance impact this would have otherwise
#85
in reply to:
↑ 84
@
5 years ago
Replying to flixos90:
I like the suggestions by @mikeschroder @peterwilsoncc @azaozz to:
- prepare a proposal post for the make blog
- create a feature plugin for easy testing
Great, let's do it! :)
Regarding relying on theme support, I'd advocate against this.
Think so too. Ideally this should be on at all times, with a way for plugins to extend it or add "conditions" for particular contexts or images.
#86
@
5 years ago
Some initial thoughts when looking at 44427.8.diff:
- Leave auto-adding of
loading="lazy"
to iframes out for now. Looking at the specs, there's no mention of iframes anywhere (unless I'm missing something): https://github.com/whatwg/html/pull/3752/files. The specs are not finalized yet, thinking we can add support for iframes when/if it becomes official. Another reason is that iframes are rarely used in user supplied content. The most common use is in markup from oEmbed. Seems better to not modify it, at least for now.
- Instead of adding
preg_replace_callback()
for img tags, abstract and reuse (or replace) the existingpreg_match_all( '/<img [^>]+>/', $content, $matches )
inwp_make_content_images_responsive()
. Ideally this should also pass some context, perhapscurrent_filter()
, but also may support some sort of a$context
param.
- I'm not particularly fond of
wp_get_lazy_load_tags()
, especially if we're not supporting iframes for now, but it makes sense to have a helper function that "holds" a filter to globally enable/disable auto-adding of theloading
attribute. (May need more thinking/reviews).
#87
@
5 years ago
FYI regarding exclusion strings; there's an initiative among developers of plugins that provide lazy-load functionality to standardize lazy-load exclusion based on skip-lazy
(class) and data-skip-lazy
(attribute, following the same naming convention as the class). The first is already in wide use as mentioned in previous comments, the attribute to some lesser extent, but this is expected to change soon. Would be great if this could be implemented in core like that as well.
#88
in reply to:
↑ 39
@
5 years ago
Replying to flixos90:
I'd also like to confirm @addyosmani's point about interoperability: Sites that currently use a JavaScript-based approach for lazy-loading won't be affected, as for these solutions there most likely won't be a
src
attribute on the images.
Late feedback; a lot of lazy-loading plugins actually do use a src
attribute to hold a "dummy image", often a data uri. This means the presence of a src
attribute does not ensure automatic interoperability.
#89
follow-up:
↓ 90
@
5 years ago
We've set up a feature plugin repository on GitHub: https://github.com/WordPress/wp-lazy-loading
The latest patch has been migrated there (in plugin form, thus sometimes using filters instead of being part of core functions). Functions that only exist for such filters have been marked with access private
.
Moving forward, let's continue the technical discussions in issues on that repository and iterate on the plugin code through pull-requests.
I'll prepare a proposal post tomorrow in a doc and share that for review, so we can hopefully publish that early next week to get feedback from a wider audience.
@azaozz Do you think we should publish this in the plugin repository too? Already now as a first version, or should we wait for initial feedback before?
#90
in reply to:
↑ 89
@
5 years ago
Replying to flixos90:
I'll prepare a proposal post tomorrow in a doc and share that for review, so we can hopefully publish that early next week to get feedback from a wider audience.
@azaozz Do you think we should publish this in the plugin repository too?
Thinking it's a bit early, perhaps. There are few things that need fixing as mentioned above. Also currently the plugin seems overly complex? As a WP plugin it's very simple. Perhaps best to keep it as simple as possible (will try to simplify it a bit before we start iterating) :)
Also thinking that ideally the post on make/core will be published at approximately the same time the plugin becomes available to install and use. Then can try to popularize it more by mentioning in dev. chat, etc.
#91
@
5 years ago
Quick update: The feature plugin and announcement post are now published.
This ticket was mentioned in Slack in #core-media by antpb. View the logs.
5 years ago
#93
@
5 years ago
- Milestone changed from 5.4 to 5.5
Moving to 5.5 as WP 5.4 Beta 1 is today (enhancements have to be committed before beta 1).
#94
@
5 years ago
- Keywords early added
With the 5.4 beta deadline being today, the goal is to refine the feature plugin over the next few weeks, get more testing, and then create a core patch. This should be merged early in the 5.5 release cycle.
Moving this to 5.5 should furthermore ensure that the WHATWG spec for lazy-loading is merged before this is included in WordPress core.
However, it should be clarified in general that it is a strict requirement for WordPress core to only ship this as the spec is merged. Even if it gets committed without the spec being merged (to get more testing), if the spec is not merged before RC phase of the WordPress release cycle, the feature should be pulled out again.
I will soon publish a follow-up on the make blog explaining in more detail.
#95
@
5 years ago
...it should be clarified in general that it is a strict requirement for WordPress core to only ship this as the spec is merged. Even if it gets committed without the spec being merged (to get more testing), if the spec is not merged before RC phase of the WordPress release cycle, the feature should be pulled out again.
Yes, that's the best way to handle this.
#96
@
5 years ago
Browser updates on native-lazyloading:
The specification for native image lazy-loading has been merged into the HTML standard as of this last week: https://github.com/whatwg/html/pull/3752#issuecomment-585202516
Firefox sent out their intent to prototype and ship support: https://groups.google.com/forum/#!msg/mozilla.dev.platform/3CH2pXxTWl4/xXc4_7QJBQAJ (demo: https://twitter.com/addyosmani/status/1228050983076290561)
Support has also landed in WebKit (TBD if this ships in a version of Safari): https://bugs.webkit.org/show_bug.cgi?id=200764
Exciting times that hopefully helps move the conversation here along... :)
#97
@
5 years ago
- Keywords needs-testing added
In 44427.9.diff: Merge patch for the https://github.com/WordPress/wp-lazy-loading plugin.
- Introduces
wp_lazy_loading_enabled()
helper function to easily enable or disable adding of the loading attribute for specific contexts. - Introduces
wp_filter_content_tags()
that adds srcset, sizes and loading attributes to img tags (for now). Will eventually be used to add loading attribute to iframes. Replaceswp_make_content_images_responsive()
. - Introduces
wp_img_tag_add_loading_attr()
andwp_img_tag_add_srcset_and_sizes_attr()
to go withwp_filter_content_tags()
. - Adds
loading="lazy"
to image tags on the_content, the_excerpt, and widget_text_content filters (front-end) by default. These can be disabled or changed by specifying aloading
attribute before the filter, by usingwp_lazy_loading_enabled()
and the specific context (the filter name is used as context), or by using thewp_img_tag_add_loading_attr
filter that runs before adding the attribute to each img tag. - Adds
loading="lazy"
towp_get_attachment_image()
andget_avatar()
by default. Can be disabled by usingwp_lazy_loading_enabled()
(context will bewp_get_attachment_image
orget_avatar
respectively), or by passing a value for the loading attribute.
Please test! :)
Alternatively the WP Lazy Loading plugin can be tested. It has identical functionality/code.
This ticket was mentioned in Slack in #core by azaozz. View the logs.
5 years ago
#99
@
5 years ago
44427.9.diff looks all good to me, turning the feature plugin into a core patch.
Just two very minor things:
- In media.php line 1600, shouldn't we use "bool" instead of "boolean" per our doc standards?
- In the same doc block, parameter description indentation is off.
With some more testing, this should be ready to go.
#100
follow-up:
↓ 102
@
5 years ago
- Type changed from feature request to defect (bug)
- Version set to 5.3.2
Plugin broke json inside post.
In my case I got:
[raw] <script>jQuery(function($) { $('#quiz').quiz({ … q: "Some text <img src='picture.jpg' width='500' height='302' class='aligncenter size-full' />", … </script> [/raw]
So when adding loading="lazy" code is broken due to double-quotes. Maybe it possible to add some exeption option. (In my case I just added loading='lazy' myself with single-quotes.)
#101
@
5 years ago
- Type changed from defect (bug) to enhancement
- Version changed from 5.3.2 to trunk
#102
in reply to:
↑ 100
@
5 years ago
Replying to narwen:
Plugin broke json inside post.
Right. Having HTML tags with single quotes for the attributes is very very rare for post_content, but not impossible. Seems we'll have to check what quotes are used. Updated patch coming up.
#103
@
5 years ago
In 44427.10.diff:
- Check which quote is used in the
img
tag and use the same for theloading
attribute. - Add a test to confirm the single quote support.
- Fix few small docblock inconsistencies.
The quote style check also checks whether the img tag is valid, i.e. it has a src
attribute.
This ticket was mentioned in Slack in #core by azaozz. View the logs.
5 years ago
#107
@
5 years ago
Just a note that in the [47554] props mukeshpanchal27
(GH user name) should be mukesh27
(WP.org user name). See https://core.trac.wordpress.org/ticket/44427#comment:105.
#112
@
4 years ago
@whyisjake I'm working on it this week, ETA with reviews etc I'd say mid next week.
#113
@
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/
#114
@
4 years ago
Just a bit of feedback about wp_filter_content_tags
specifically...
It is the suggested replacement for wp_make_content_images_responsive
, but the new function name appears to not be image-specific. My interpretation of the new function name is that it may, in the future, be responsible for filtering all content tags.
bbPress uses wp_make_content_images_responsive
, but I'm uncertain if wp_filter_content_tags
will be a suitable long-term replacement, because community submitted content has different needs than Contributor-and-higher content.
Subjectively, there was something nice about isolating image-specific functionality previous to 5.5. If the intent is to add more tags to this in the future, bbPress may need to fork this. If that is not the intent, the new function name might be more ambiguous than intended.
I'm not fluent with the history or planning here, so I apologize if I'm mistaken. ☮️
This ticket was mentioned in Slack in #core-media by antpb. View the logs.
4 years ago
#116
@
4 years ago
I would like to second @johnjamesjacoby's question about the naming of wp_filter_content_tags. This function does appear to be targeting img tags only.
#117
@
4 years ago
@johnjamesjacoby, @johnstonphilip that's a good question :)
The original proposal for the loading
attribute was for img
and iframe
tags. It was added first for img
. Now it's coming to iframe
as well, see https://github.com/whatwg/html/pull/5579. Hoping in 5.6 WP will add support for iframes with the already existing functionality. Hence the naming of these functions and filters to allow additions in the future.
#118
follow-up:
↓ 119
@
4 years ago
@azaozz Thanks for clarifying that. I am happy to be wrong here, but the function name being purpose-agnostic in this way lends itself to growing and changing in scope over time. Are there any proposed limits on what this function might do to tags in the_content in the future? Is it for optimization purposes only?
Or should it be expected that this function could change any tag in any way, as new things like these are introduced? Is this function a catch all for new attribute-related tech?
#119
in reply to:
↑ 118
;
follow-up:
↓ 120
@
4 years ago
Replying to johnstonphilip:
Are there any proposed limits on what this function might do...
This function (and its predecessor) have been used only for back-fills of new, significant HTML features to existing posts/content. Don't think that will change.
At the same time, it's "just" a function's name. Nice to be descriptive and self-documenting, but at the end of the day doesn't define how a function works (naming of functions and vars is the hardest part in coding!) :)
#120
in reply to:
↑ 119
@
4 years ago
Replying to johnstonphilip:
Are there any proposed limits on what this function might do...
This function (and its predecessor) have been used only for back-fills of new, significant HTML features to existing posts/content. Don't think that will change.
To add to that, this function is more generic than its predecessor to allow for the existing and future cases where we'll need to back-fill attributes to happen in a more performant way (e.g. combined regex). If a plugin would like fine-grained control (e.g. lazy-load images but not add responsive attributes), it should preferably not unhook the entire function, but use the more specific filters available.
If a plugin has its own special type of content that it would like to receive similar treatment for, that's probably the only case that this function should be manually called.
#121
follow-up:
↓ 122
@
4 years ago
@azaozz Would it be possible to describe the function's purpose with more clarity in the docblock's description?
While the current description is good, I wonder if it might help future developers wondering where to put similar new HTML features, and also help to set the expectation for people who might use this function for various purposes.
Here's one possible iteration of a description for the docblock:
/** * Modifies specific HTML tags in post content to include new browser and HTML technologies * that may not have existed at the time of post creation. These modifications currently include * adding `srcset`, `sizes`, and `loading` attributes to `img` HTML tags. Future similar * optimizations should be added/expected here. * * @since 5.5.0 * * @see wp_img_tag_add_width_and_height_attr() * @see wp_img_tag_add_srcset_and_sizes_attr() * @see wp_img_tag_add_loading_attr() * * @param string $content The HTML content to be filtered. * @param string $context Optional. Additional context to pass to the filters. * Defaults to `current_filter()` when not set. * @return string Converted content with images modified. */ function wp_filter_content_tags( $content, $context = null ) {
#122
in reply to:
↑ 121
@
4 years ago
Replying to johnstonphilip:
Sure, sounds good.
#126
in reply to:
↑ 16
@
4 years ago
Replying to mor10:
Hey Morten, from what I understand, the loading="lazy" attribute is supposed to stop images out of the viewport from being loaded...as this video demonstrates: https://www.youtube.com/watch?v=-UaKADF8SN4
I have tried a few browsers besides Chrome and it looks like images are all loaded at once when I scroll down. Does connection speed have any bearing on this?
Lazy loading was created specifically to address page load for people on slower connections because of large page sizes mainly caused by images appearing "below the fold" and out of view. There's ample research to show people rarely scroll all the way down a page, and loading images out of view is a waste of valuable resources which incurs cost and slowness for the end-user, especially on slow connections. By lazy loading images and other heavy assets, the user is delivered a significantly improved user experience while at the same time saving both time and money.
The specific spec from Google, which is now implemented in Chrome, is to use the
loading
attribute to instruct the browser on how to lazy load images and iframes. By default, the browser treats all images and iframes asloading="auto"
meaning the browser automatically lazy loads assets based on its own formula. Developers can set each asset to eitherloading="lazy"
which "defers loading of the resource until it reaches a calculated distance from the viewport", orloading="eager"
which "loads the resource immediately, regardless of where it's located on the page."
As the developer of a performance optimization plugin, I definitely relate with points 3 and 4, which are recurring issues for both our development & support teams.
Having a standardized lazy-loading API in core would greatly reduce the conflicts we encounter, and reduce our need to consistently & accurately parse the HTML content of pages to detect and rewrite lazy-loadable media assets.
I believe it would also help solve difficult situations like lazy-loading background images, a common user-requested feature, which is hard to get around without a core standard.