Make WordPress Core

Opened 6 years ago

Closed 4 years ago

Last modified 3 years ago

#44427 closed enhancement (fixed)

Introduce lazy-loading API for media and other elements

Reported by: mor10's profile mor10 Owned by: flixos90's profile 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:

  1. Lazy-loading is performance best-practice: https://developers.google.com/web/fundamentals/performance/lazy-loading-guidance/images-and-video/.
  2. WordPress should not put the onus on site-owners to have in-depth knowledge of performance best-practices and how to meet them.
  3. 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.
  4. Lazy-loading introduced at plugin/theme level forces aggressive rewriting of content which would be more consistently performed at core.

Attachments (11)

lazy-load-theme-screenshots.patch (4.6 KB) - added by pierlo 5 years ago.
44427.diff (2.6 KB) - added by spacedmonkey 5 years ago.
44427.2.diff (3.2 KB) - added by spacedmonkey 5 years ago.
44427.3.diff (2.8 KB) - added by flixos90 4 years ago.
44427.4.diff (6.9 KB) - added by flixos90 4 years ago.
44427.5.diff (8.8 KB) - added by spacedmonkey 4 years ago.
44427.6.diff (8.3 KB) - added by spacedmonkey 4 years ago.
44427.7.diff (9.7 KB) - added by flixos90 4 years ago.
44427.8.diff (10.8 KB) - added by spacedmonkey 4 years ago.
44427.9.diff (24.6 KB) - added by azaozz 4 years ago.
44427.10.diff (25.3 KB) - added by azaozz 4 years ago.

Download all attachments as: .zip

Change History (138)

#1 @tabrisrp
6 years ago

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.

#2 follow-up: @jonoaldersonwp
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:

  1. 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 on the_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.

  1. 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).
  1. Theme/template images which use wp_get_attachment_image_srcset() or similar to construct the output of the img/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!

Last edited 6 years ago by jonoaldersonwp (previous) (diff)

#3 in reply to: ↑ 2 @flixos90
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 the intersection 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 about title-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 to wp_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 and wp_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 inline style 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 @jonoaldersonwp
6 years ago

  • Agreed on requiring explicit theme support.
  • Agreed on only catching common/reasonable use-cases (e.g., <img> tags and get_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 with lazy-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 on data-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 @tabrisrp
6 years ago

Here is a list of functions we identified, which outputs HTML for images and have an available filter:

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 @jonoaldersonwp
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.

Last edited 6 years ago by jonoaldersonwp (previous) (diff)

#7 @tabrisrp
5 years ago

How can we help this move forward?

#8 follow-up: @mor10
5 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.

#9 in reply to: ↑ 8 @jonoaldersonwp
5 years ago

Agreed.

#10 @tabrisrp
5 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.

Last edited 5 years ago by tabrisrp (previous) (diff)

#11 @spacedmonkey
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 @pcfreak30
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 @mor10
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!

https://web.dev/native-lazy-loading

#14 @jonoaldersonwp
5 years ago

Yep! How? :P

#15 @JakePT
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).

Last edited 5 years ago by JakePT (previous) (diff)

#16 follow-up: @mor10
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 or eager to control how they are loaded. A good example would be featured images being set to eager because they typically appear at the top.
  • Users could be provided with a toggle to choose if they want individual images to load lazy or eager, 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:

  1. 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.
  2. 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: @pierlo
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: @jonoaldersonwp
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 transform img.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 @willstockstech
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 @willstockstech
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 @jonoaldersonwp
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.

Last edited 5 years ago by jonoaldersonwp (previous) (diff)

#24 follow-ups: @mor10
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.

#25 @jonoaldersonwp
5 years ago

What @mor10 said.

Last edited 5 years ago by jonoaldersonwp (previous) (diff)

#26 in reply to: ↑ 24 @marsjaninzmarsa
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 @willstockstech
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 @pierlo
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'?

#29 @drywallbmb
5 years ago

Love the idea. Let's make it happen!

@spacedmonkey
5 years ago

#30 @spacedmonkey
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.

@spacedmonkey
5 years ago

#31 @birgire
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.

Last edited 5 years ago by birgire (previous) (diff)

#32 @jonoaldersonwp
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 @birgire
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 @jonoaldersonwp
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 @birgire
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 the jetpack_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

Last edited 5 years ago by birgire (previous) (diff)

#36 @jonoaldersonwp
5 years ago

Gah. Trust those two to have frustrating anti-patterns.
Ok, yeah, we should sniff for those!

#37 @mor10
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 @addyosmani
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: @flixos90
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 @mor10
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 @smub
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.

#42 @swissspidy
5 years ago

#48056 was marked as a duplicate.

@flixos90
4 years ago

#43 @flixos90
4 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 @westonruter
4 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: @flixos90
4 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 after convert_smilies which runs at priority 20 (at least for the_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.

@flixos90
4 years ago

#46 @flixos90
4 years ago

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

44427.4.diff adds unit test coverage.

#47 @swissspidy
4 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 @flixos90
4 years ago

@swissspidy

Might make sense to add the filter to render_block in addition to widget_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 @swissspidy
4 years ago

I'm not sure about render_block, since that content would usually be covered already by the_content right?

True, unless the block is displayed in a widget area or block area.

#50 in reply to: ↑ 45 @westonruter
4 years ago

Replying to flixos90:

Also, I believe wp_lazy_load_content_media should be run after convert_smilies which runs at priority 20 (at least for the_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.

@spacedmonkey
4 years ago

#51 follow-up: @spacedmonkey
4 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 @westonruter
4 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.

@spacedmonkey
4 years ago

#53 @spacedmonkey
4 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 to wp_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 @spacedmonkey
4 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.

#56 @spacedmonkey
4 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 @flixos90
4 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 @spacedmonkey
4 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 @flixos90
4 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 @spacedmonkey
4 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.


4 years ago

@flixos90
4 years ago

#62 @flixos90
4 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 include iframe tags). I think only supporting img tags initially by default is a safer bet, and also way more important because of how much they're used - we can revisit iframe 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.


4 years ago

#64 @spacedmonkey
4 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.


4 years ago

#66 @kirasong
4 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: @spacedmonkey
4 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.

@spacedmonkey
4 years ago

#68 @spacedmonkey
4 years ago

44427.8.diff

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: @azaozz
4 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: @flixos90
4 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 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?

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 @azaozz
4 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 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.

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 and decoding), 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.

Last edited 4 years ago by azaozz (previous) (diff)

#72 @spacedmonkey
4 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.


4 years ago

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


4 years ago

#75 in reply to: ↑ 67 ; follow-up: @kirasong
4 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: @westonruter
4 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 @azaozz
4 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 a 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.

Version 1, edited 4 years ago by azaozz (previous) (next) (diff)

#78 in reply to: ↑ 76 @azaozz
4 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's uploads 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?

Last edited 4 years ago by azaozz (previous) (diff)

#79 in reply to: ↑ 76 ; follow-ups: @peterwilsoncc
4 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'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.

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 @westonruter
4 years ago

Replying to peterwilsoncc:

Replying to westonruter:
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.

This shouldn't apply because loading=lazy only affects behavior of images that are outside the viewport.

#81 in reply to: ↑ 79 ; follow-up: @azaozz
4 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 :)

Last edited 4 years ago by azaozz (previous) (diff)

#82 in reply to: ↑ 81 @peterwilsoncc
4 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 @spacedmonkey
4 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: @flixos90
4 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 @azaozz
4 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 @azaozz
4 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 existing preg_match_all( '/<img [^>]+>/', $content, $matches ) in wp_make_content_images_responsive(). Ideally this should also pass some context, perhaps current_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 the loading attribute. (May need more thinking/reviews).

#87 @OptimizingMatters
4 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 @OptimizingMatters
4 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: @flixos90
4 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 @azaozz
4 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 @flixos90
4 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.


4 years ago

#93 @audrasjb
4 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 @flixos90
4 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 @azaozz
4 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.

Last edited 4 years ago by azaozz (previous) (diff)

#96 @addyosmani
4 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... :)

@azaozz
4 years ago

#97 @azaozz
4 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. Replaces wp_make_content_images_responsive().
  • Introduces wp_img_tag_add_loading_attr() and wp_img_tag_add_srcset_and_sizes_attr() to go with wp_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 a loading attribute before the filter, by using wp_lazy_loading_enabled() and the specific context (the filter name is used as context), or by using the wp_img_tag_add_loading_attr filter that runs before adding the attribute to each img tag.
  • Adds loading="lazy" to wp_get_attachment_image() and get_avatar() by default. Can be disabled by using wp_lazy_loading_enabled() (context will be wp_get_attachment_image or get_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.

Last edited 4 years ago by azaozz (previous) (diff)

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


4 years ago

#99 @flixos90
4 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: @narwen
4 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.)

Last edited 4 years ago by narwen (previous) (diff)

#101 @spacedmonkey
4 years ago

  • Type changed from defect (bug) to enhancement
  • Version changed from 5.3.2 to trunk

#102 in reply to: ↑ 100 @azaozz
4 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.

@azaozz
4 years ago

#103 @azaozz
4 years ago

In 44427.10.diff:

  • Check which quote is used in the img tag and use the same for the loading 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.


4 years ago

#105 @azaozz
4 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 47554:

Media: Enable lazy-loading of images by automatically adding the new loading="lazy" attribute to image tags on the front-end.

  • Introduces wp_lazy_loading_enabled(), wp_filter_content_tags(), wp_img_tag_add_loading_attr(), and wp_img_tag_add_srcset_and_sizes_attr() functions.
  • Introduces wp_lazy_loading_enabled, wp_img_tag_add_loading_attr, and wp_img_tag_add_srcset_and_sizes_attr filters.

Props flixos90, addyosmani, mor10, swissspidy, pierlo, westonruter, spacedmonkey, mikeschroder, jonoaldersonwp, peterwilsoncc, narwen, jeffpaul, OptimizingMatters, futtta, mukeshpanchal27, azaozz.

Fixes #44427.

Last edited 4 years ago by azaozz (previous) (diff)

#106 @joemcgill
4 years ago

  • Keywords needs-dev-note added

#107 @azaozz
4 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.

#108 @flixos90
4 years ago

In 48170:

Media: Ensure images have dimensions to reduce layout shift and facilitate lazy-loading.

This changeset ensures that attachment images which are inserted without width and height attributes still receive them in the frontend, to reduce cumulative layout shift. Adding the dimensions happens as part of the logic for adding srcset and sizes attributes, which already assume the specific width and height of the respective image.

Images are now only lazy-loaded if they have width and height attributes present. While missing these attributes itself is what causes layout shifts, lazy-loading such images can make this problem more apparent to the user.

Props adamsilverstein, westonruter.
Fixes #50367. See #44427.

#109 @flixos90
4 years ago

In 48237:

Media: Introduce wp_img_tag_add_width_and_height_attr() to add dimension attributes to images.

Following up on [48170], this changeset moves the new logic to add missing img dimension attributes into a separate function that is run first within wp_filter_content_tags(). It also adds a utility function wp_image_src_get_dimensions() with logic reused from wp_image_add_srcset_and_sizes(), and it ensures that width and height attributes only get added if both of the attributes are missing on the original img tag.

This changeset furthermore improves test coverage and separates tests for the different aspects of img tag modification.

Props azaozz.
Fixes #50367. See #44427.

#110 @flixos90
4 years ago

In 48272:

Media: Improve support for opting out of lazy-loading for template images.

With this changeset, in addition to the already present wp_lazy_loading_enabled filter, developers can now opt out of lazy-loading template images via wp_get_attachment_image() by passing a loading attribute with boolean value false. This can be used e.g. by theme developers on images which are very likely to be in the initial viewport.

This changeset also improves related test coverage.

Props adamsilverstein, azaozz, joemcgill, johnbillion.
See #50425, #44427.

#111 @whyisjake
4 years ago

What's the status on getting a dev note for lazy loading?

#112 @flixos90
4 years ago

@whyisjake I'm working on it this week, ETA with reviews etc I'd say mid next week.

#113 @flixos90
4 years ago

  • Keywords has-dev-note added; needs-dev-note removed

#114 @johnjamesjacoby
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 @johnstonphilip
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 @azaozz
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.

Last edited 4 years ago by azaozz (previous) (diff)

#118 follow-up: @johnstonphilip
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: @azaozz
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!) :)

Last edited 4 years ago by azaozz (previous) (diff)

#120 in reply to: ↑ 119 @flixos90
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: @johnstonphilip
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 @azaozz
4 years ago

Replying to johnstonphilip:

Sure, sounds good.

#123 @azaozz
4 years ago

In 48648:

Media: Improve inline docs for wp_filter_content_tags().

Props johnstonphilip.
See #44427.

#124 @johnstonphilip
4 years ago

@azaozz Thanks! :)

#125 @SergeyBiryukov
4 years ago

In 48649:

Docs: Correct description for wp_filter_content_tags() per the documentation standards.

DocBlocks should start with a brief, one sentence explanation of the purpose of the function spanning a maximum of two lines.

Follow-up to [48648].

See #44427.

#126 in reply to: ↑ 16 @captainmoe
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 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."

#127 @archon810
3 years ago

Looks like lazy-loading of iframes is being tracked in #50756 and is currently slated for 5.7, not 5.6.

Note: See TracTickets for help on using tickets.