Changeset 55825

Timestamp:

05/18/2023 07:53:37 PM (19 months ago)

Author:

flixos90

Message:

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.

Props flixos90, antpb, joedolson, spacedmonkey, mukesh27, thekt12, costdev, jrf.
Fixes #58089.
See #53675.

Location:

trunk

Files:

• src/wp-includes/media.php (modified) (1 diff)
• tests/phpunit/tests/media.php (modified) (1 diff)

trunk/src/wp-includes/media.php

@@ -5508 +5508 @@
 
     /*
+     * 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.

trunk/tests/phpunit/tests/media.php

@@ -3854 +3854 @@
     }
 
+    /**
+     * @ticket 58089
+     *
+     * @covers ::wp_filter_content_tags
+     * @covers ::wp_get_loading_attr_default
+     */
+    public function test_wp_filter_content_tags_does_not_lazy_load_special_images_within_the_content() {
+        global $wp_query, $wp_the_query;
+
+        // Force no lazy-loading on the image tag expected in the content.
+        $expected_content = wpautop( wp_get_attachment_image( self::$large_id, 'large', false, array( 'loading' => false ) ) );
+
+        // Overwrite post content with an image.
+        add_filter(
+            'the_content',
+            static function() {
+                // Replace content with an image tag, i.e. the 'wp_get_attachment_image' context is used while running 'the_content' filter.
+                return wp_get_attachment_image( self::$large_id, 'large', false );
+            },
+            9 // Run before wp_filter_content_tags().
+        );
+
+        /*
+         * We have to run a main query loop so that the first 'the_content' context image is not
+         * lazy-loaded.
+         * Without the fix from 58089, the image would still be lazy-loaded since the check for the
+         * separately invoked 'wp_get_attachment_image' context would lead to that.
+         */
+        $wp_query     = new WP_Query( array( 'post__in' => array( self::$post_ids['publish'] ) ) );
+        $wp_the_query = $wp_query;
+        $this->reset_content_media_count();
+        $this->reset_omit_loading_attr_filter();
+        $content = '';
+        while ( have_posts() ) {
+            the_post();
+            $content = get_echo( 'the_content' );
+        }
+
+        // Ensure that parsed content has the image without lazy-loading.
+        $this->assertSame( $expected_content, $content );
+    }
+
+    /**
+     * Tests that wp_get_loading_attr_default() returns 'lazy' for special contexts when they're used outside of 'the_content' filter.
+     *
+     * @ticket 58089
+     *
+     * @covers ::wp_get_loading_attr_default
+     *
+     * @dataProvider data_special_contexts_for_the_content
+     *
+     * @param string $context Context for the element for which the `loading` attribute value is requested.
+     */
+    public function test_wp_get_loading_attr_default_should_return_lazy_for_special_contexts_outside_of_the_content( $context ) {
+        $this->assertSame( 'lazy', wp_get_loading_attr_default( $context ) );
+    }
+
+    /**
+     * Tests that wp_get_loading_attr_default() returns false for special contexts when they're used within 'the_content' filter.
+     *
+     * @ticket 58089
+     *
+     * @covers ::wp_get_loading_attr_default
+     *
+     * @dataProvider data_special_contexts_for_the_content
+     *
+     * @param string $context Context for the element for which the `loading` attribute value is requested.
+     */
+    public function test_wp_get_loading_attr_default_should_return_false_for_special_contexts_within_the_content( $context ) {
+        remove_all_filters( 'the_content' );
+
+        $result = null;
+        add_filter(
+            'the_content',
+            function( $content ) use ( &$result, $context ) {
+                $result = wp_get_loading_attr_default( $context );
+                return $content;
+            }
+        );
+        apply_filters( 'the_content', '' );
+        $this->assertFalse( $result );
+    }
+
+    /**
+     * Data provider.
+     *
+     * @return array[]
+     */
+    public function data_special_contexts_for_the_content() {
+        return array(
+            'the_post_thumbnail'      => array( 'context' => 'the_post_thumbnail' ),
+            'wp_get_attachment_image' => array( 'context' => 'wp_get_attachment_image' ),
+        );
+    }
+
     private function reset_content_media_count() {
         // Get current value without increasing.