Skip to content
This repository has been archived by the owner on Mar 9, 2024. It is now read-only.

Commit

Permalink
WP-r55850: Media: Fix bug in lazy-loading of images in excerpts (#274)
Browse files Browse the repository at this point in the history
* WP-r55816: Media: Increase default for `wp_omit_loading_attr_threshold` to 3.

The previous default threshold for how many content images to skip lazy-loading on (which was just 1) has proven to be too strict: HTTP Archive data shows that >70% of sites have up to 3 equal-sized images in the initial viewport, each of which could be the LCP image and therefore should not be lazy-loaded. Lazy-loading too many images has adverse effects on load time performance, while increasing the default threshold will not negatively affect load time performance even for sites where a threshold of 1 would be the perfect choice.

The change of default value in this changeset will improve performance for more WordPress sites out of the box. The `wp_omit_loading_attr_threshold` filter can still be used to customize and fine tune the value where needed.

WP:Props thekt12, spacedmonkey, westonruter, flixos90.
Fixes https://core.trac.wordpress.org/ticket/58213.

Conflicts:
- tests/phpunit/tests/media.php

---

Merges https://core.trac.wordpress.org/changeset/55816 / WordPress/wordpress-develop@a56a83fc6c to ClassicPress.

* Fix merge conflicts from 55816

* WP-r55847: Media: Conditionally skip lazy-loading on images before the loop to improve LCP performance.

When the logic to exclude images that likely appear above the fold from being lazy-loaded was introduced in WordPress 5.9, initially only images that appear within the main query loop were being considered. However, there is a good chance that images above the fold are rendered before the loop starts, for example in the header template part.

It is particularly common for a theme to display the featured image for a single post in the header. Based on HTTP Archive data from February 2023, the majority of LCP images that are still being lazy-loaded on WordPress sites use the `wp-post-image` class, i.e. are featured images.

This changeset enhances the logic in `wp_get_loading_attr_default()` to not lazy-load images that appear within or after the header template part and before the query loop, using a new `WP_Query::$before_loop` property.

For block themes, this was for the most part already addressed in https://core.trac.wordpress.org/changeset/55318, however this enhancement implements the solution in a more generally applicable way that brings the improvement to classic themes as well.

WP:Props thekt12, flixos90, spacedmonkey, costdev, zunaid321, mukesh27.
Fixes https://core.trac.wordpress.org/ticket/58211.
See https://core.trac.wordpress.org/ticket/53675, https://core.trac.wordpress.org/ticket/56930.

Conflicts:
- src/wp-includes/media.php
- tests/phpunit/tests/media.php

---

Merges https://core.trac.wordpress.org/changeset/55847 / WordPress/wordpress-develop@71140f327f to ClassicPress.

* Fix merge conflicts from 55847

* WP-r55850: Media: Fix lazy-loading bug by avoiding to modify content images when creating an excerpt.

The `wp_filter_content_tags()` function, which modifies image tags for example to optimize performance, is hooked into the `the_content` filter by default. When rendering an excerpt for a post that doesn't have a manually provided excerpt, the post content is used to generate the excerpt, handled by the `wp_trim_excerpt()` function.

Prior to this changeset, this led to `wp_filter_content_tags()` being called on the content when generating the excerpt, which is wasteful as all tags are stripped from the excerpt, and it furthermore could result in a lazy-loading bug when the post content contained images, as those images were being counted even though they would never be rendered as part of the excerpt.

This changeset fixes the bug and slightly improves performance for generating an excerpt by temporarily unhooking the `wp_filter_content_tags()` function from the `the_content` filter when using it to generate the excerpt.

WP:Props costdev, flixos90, joemcgill, mukesh27, salvoaranzulla, spacedmonkey, thekt12, westonruter.
Fixes https://core.trac.wordpress.org/ticket/56588.

Conflicts:
- tests/phpunit/tests/media.php

---

Merges https://core.trac.wordpress.org/changeset/55850 / WordPress/wordpress-develop@6ff355e87d to ClassicPress.

* Fix merge conflicts in 55850

* WP-r55825: Media: Prevent special images within post content to skew image counts and cause lazy-loading bugs.

In order to skip lazy-loading the first few images on a page, as of WordPress 5.9 there has been logic to count images that are eligible based on certain criteria. One of those groups are images that appear within the content of a post.

This changeset fixes a bug where images created via `get_the_post_thumbnail()` or `wp_get_attachment_image()` that are injected into the post content would skew the count and therefore result in all images to be lazy-loaded, potentially hurting load time performance. This is relevant for example when those functions are called in server-side rendered blocks, or any other filter callbacks hooked into `the_content`.

WP:Props flixos90, antpb, joedolson, spacedmonkey, mukesh27, thekt12, costdev, jrf.
Fixes https://core.trac.wordpress.org/ticket/58089.
See https://core.trac.wordpress.org/ticket/53675.

---

Merges https://core.trac.wordpress.org/changeset/55825 / WordPress/wordpress-develop@23b007b126 to ClassicPress.

* Removed duplicated test from 55825

---------

Co-authored-by: Felix Arntz <[email protected]>
  • Loading branch information
mattyrob and felixarntz authored Nov 12, 2023
1 parent 360e539 commit 8a12a49
Show file tree
Hide file tree
Showing 6 changed files with 583 additions and 50 deletions.
12 changes: 12 additions & 0 deletions src/wp-includes/class-wp-query.php
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,14 @@ class WP_Query {
*/
public $current_post = -1;

/**
* Whether the caller is before the loop.
*
* @since 6.3.0
* @var bool
*/
public $before_loop = true;

/**
* Whether the loop has started and the caller is in the loop.
*
Expand Down Expand Up @@ -517,6 +525,7 @@ public function init() {
$this->post_count = 0;
$this->current_post = -1;
$this->in_the_loop = false;
$this->before_loop = true;
unset( $this->request );
unset( $this->post );
unset( $this->comments );
Expand Down Expand Up @@ -3631,6 +3640,7 @@ public function the_post() {
}

$this->in_the_loop = true;
$this->before_loop = false;

if ( -1 == $this->current_post ) { // Loop has just started.
/**
Expand Down Expand Up @@ -3671,6 +3681,8 @@ public function have_posts() {
// Do some cleaning up after the loop.
$this->rewind_posts();
} elseif ( 0 === $this->post_count ) {
$this->before_loop = false;

/**
* Fires if no results are found in a post query.
*
Expand Down
16 changes: 16 additions & 0 deletions src/wp-includes/formatting.php
Original file line number Diff line number Diff line change
Expand Up @@ -3923,10 +3923,26 @@ function wp_trim_excerpt( $text = '', $post = null ) {

$text = strip_shortcodes( $text );

/*
* Temporarily unhook wp_filter_content_tags() since any tags
* within the excerpt are stripped out. Modifying the tags here
* is wasteful and can lead to bugs in the image counting logic.
*/
$filter_removed = remove_filter( 'the_content', 'wp_filter_content_tags' );

/** This filter is documented in wp-includes/post-template.php */
$text = apply_filters( 'the_content', $text );
$text = str_replace( ']]>', ']]&gt;', $text );

/**
* Only restore the filter callback if it was removed above. The logic
* to unhook and restore only applies on the default priority of 10,
* which is generally used for the filter callback in WordPress core.
*/
if ( $filter_removed ) {
add_filter( 'the_content', 'wp_filter_content_tags' );
}

/* translators: Maximum number of words used in a post excerpt. */
$excerpt_length = (int) _x( '55', 'excerpt_length' );

Expand Down
50 changes: 46 additions & 4 deletions src/wp-includes/media.php
Original file line number Diff line number Diff line change
Expand Up @@ -5336,16 +5336,20 @@ function wp_get_webp_info( $filename ) {
*
* Under the hood, the function uses {@see wp_increase_content_media_count()} every time it is called for an element
* within the main content. If the element is the very first content element, the `loading` attribute will be omitted.
* This default threshold of 1 content element to omit the `loading` attribute for can be customized using the
* This default threshold of 3 content elements to omit the `loading` attribute for can be customized using the
* {@see 'wp_omit_loading_attr_threshold'} filter.
*
* @since 5.9.0
*
* @global WP_Query $wp_query WordPress Query object.
*
* @param string $context Context for the element for which the `loading` attribute value is requested.
* @return string|bool The default `loading` attribute value. Either 'lazy', 'eager', or a boolean `false`, to indicate
* that the `loading` attribute should be skipped.
*/
function wp_get_loading_attr_default( $context ) {
global $wp_query;

// Skip lazy-loading for the overall block template, as it is handled more granularly.
if ( 'template' === $context ) {
return false;
Expand All @@ -5357,6 +5361,43 @@ function wp_get_loading_attr_default( $context ) {
return false;
}

// Special handling for programmatically created image tags.
if ( ( 'the_post_thumbnail' === $context || 'wp_get_attachment_image' === $context ) ) {
/*
* Skip programmatically created images within post content as they need to be handled together with the other
* images within the post content.
* Without this clause, they would already be counted below which skews the number and can result in the first
* post content image being lazy-loaded only because there are images elsewhere in the post content.
*/
if ( doing_filter( 'the_content' ) ) {
return false;
}

// Conditionally skip lazy-loading on images before the loop.
if (
// Only apply for main query but before the loop.
$wp_query->before_loop && $wp_query->is_main_query()
/*
* Any image before the loop, but after the header has started should not be lazy-loaded,
* except when the footer has already started which can happen when the current template
* does not include any loop.
*/
&& did_action( 'get_header' ) && ! did_action( 'get_footer' )
) {
return false;
}
}

/*
* Skip programmatically created images within post content as they need to be handled together with the other
* images within the post content.
* Without this clause, they would already be counted below which skews the number and can result in the first
* post content image being lazy-loaded only because there are images elsewhere in the post content.
*/
if ( ( 'the_post_thumbnail' === $context || 'wp_get_attachment_image' === $context ) && doing_filter( 'the_content' ) ) {
return false;
}

/*
* The first elements in 'the_content' or 'the_post_thumbnail' should not be lazy-loaded,
* as they are likely above the fold.
Expand Down Expand Up @@ -5386,7 +5427,7 @@ function wp_get_loading_attr_default( $context ) {
/**
* Gets the threshold for how many of the first content media elements to not lazy-load.
*
* This function runs the {@see 'wp_omit_loading_attr_threshold'} filter, which uses a default threshold value of 1.
* This function runs the {@see 'wp_omit_loading_attr_threshold'} filter, which uses a default threshold value of 3.
* The filter is only run once per page load, unless the `$force` parameter is used.
*
* @since 5.9.0
Expand All @@ -5407,10 +5448,11 @@ function wp_omit_loading_attr_threshold( $force = false ) {
* for only the very first content media element.
*
* @since 5.9.0
* @since 6.3.0 The default threshold was changed from 1 to 3.
*
* @param int $omit_threshold The number of media elements where the `loading` attribute will not be added. Default 1.
* @param int $omit_threshold The number of media elements where the `loading` attribute will not be added. Default 3.
*/
$omit_threshold = apply_filters( 'wp_omit_loading_attr_threshold', 1 );
$omit_threshold = apply_filters( 'wp_omit_loading_attr_threshold', 3 );
}

return $omit_threshold;
Expand Down
56 changes: 56 additions & 0 deletions tests/phpunit/tests/formatting/WpTrimExcerpt.php
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,60 @@ public function test_should_generate_excerpt_for_empty_values() {
$this->assertSame( 'Post content', wp_trim_excerpt( null, $post ) );
$this->assertSame( 'Post content', wp_trim_excerpt( false, $post ) );
}

/**
* Tests that `wp_trim_excerpt()` unhooks `wp_filter_content_tags()` from 'the_content' filter.
*
* @ticket 56588
*/
public function test_wp_trim_excerpt_unhooks_wp_filter_content_tags() {
$post = self::factory()->post->create();

/*
* Record that during 'the_content' filter run by wp_trim_excerpt() the
* wp_filter_content_tags() callback is not used.
*/
$has_filter = true;
add_filter(
'the_content',
static function ( $content ) use ( &$has_filter ) {
$has_filter = has_filter( 'the_content', 'wp_filter_content_tags' );
return $content;
}
);

wp_trim_excerpt( '', $post );

$this->assertFalse( $has_filter, 'wp_filter_content_tags() was not unhooked in wp_trim_excerpt()' );
}

/**
* Tests that `wp_trim_excerpt()` doesn't permanently unhook `wp_filter_content_tags()` from 'the_content' filter.
*
* @ticket 56588
*/
public function test_wp_trim_excerpt_should_not_permanently_unhook_wp_filter_content_tags() {
$post = self::factory()->post->create();

wp_trim_excerpt( '', $post );

$this->assertSame( 10, has_filter( 'the_content', 'wp_filter_content_tags' ), 'wp_filter_content_tags() was not restored in wp_trim_excerpt()' );
}

/**
* Tests that `wp_trim_excerpt()` doesn't restore `wp_filter_content_tags()` if it was previously unhooked.
*
* @ticket 56588
*/
public function test_wp_trim_excerpt_does_not_restore_wp_filter_content_tags_if_previously_unhooked() {
$post = self::factory()->post->create();

// Remove wp_filter_content_tags() from 'the_content' filter generally.
remove_filter( 'the_content', 'wp_filter_content_tags' );

wp_trim_excerpt( '', $post );

// Assert that the filter callback was not restored after running 'the_content'.
$this->assertFalse( has_filter( 'the_content', 'wp_filter_content_tags' ) );
}
}
Loading

0 comments on commit 8a12a49

Please sign in to comment.