Changeset 56693

Timestamp:

09/26/2023 12:11:06 AM (18 months ago)

Author:

flixos90

Message:

Media: Ensure images within shortcodes are correctly considered for loading optimization attributes.

Prior to this change, images added in shortcodes would be considered separately from all other images within post content, which led to incorrect application of the loading optimization attributes loading="lazy" and fetchpriority="high".

This changeset changes the filter priority of wp_filter_content_tags() from the default 10 to 12 on the various content filters it is hooked in, in order to run that function after parsing shortcodes. While this may technically be considered a backward compatibility break, substantial research and lack of any relevant usage led to the assessment that the change is acceptable given its benefits.

An additional related fix included is that now the duplicate processing of images is prevented not only for post content blobs (the_content filter), but also for widget content blobs (widget_text_content and widget_block_content filters).

Props joemcgill, mukesh27, costdev, spacedmonkey, flixos90.
Fixes #58853.

Location:

trunk

Files:

• src/wp-includes/default-filters.php (modified) (4 diffs)
• src/wp-includes/formatting.php (modified) (2 diffs)
• src/wp-includes/media.php (modified) (1 diff)
• tests/phpunit/tests/formatting/wpTrimExcerpt.php (modified) (2 diffs)
• tests/phpunit/tests/media.php (modified) (2 diffs)

trunk/src/wp-includes/default-filters.php

@@ -196 +196 @@
 add_filter( 'the_content', 'shortcode_unautop' );
 add_filter( 'the_content', 'prepend_attachment' );
-add_filter( 'the_content', 'wp_filter_content_tags' );
 add_filter( 'the_content', 'wp_replace_insecure_home_url' );
+add_filter( 'the_content', 'do_shortcode', 11 ); // AFTER wpautop().
+add_filter( 'the_content', 'wp_filter_content_tags', 12 ); // Runs after do_shortcode().
 
 add_filter( 'the_excerpt', 'wptexturize' );
@@ -204 +205 @@
 add_filter( 'the_excerpt', 'wpautop' );
 add_filter( 'the_excerpt', 'shortcode_unautop' );
-add_filter( 'the_excerpt', 'wp_filter_content_tags' );
 add_filter( 'the_excerpt', 'wp_replace_insecure_home_url' );
+add_filter( 'the_excerpt', 'wp_filter_content_tags', 12 );
 add_filter( 'get_the_excerpt', 'wp_trim_excerpt', 10, 2 );
 
@@ -231 +232 @@
 add_filter( 'widget_text_content', 'wpautop' );
 add_filter( 'widget_text_content', 'shortcode_unautop' );
-add_filter( 'widget_text_content', 'wp_filter_content_tags' );
 add_filter( 'widget_text_content', 'wp_replace_insecure_home_url' );
 add_filter( 'widget_text_content', 'do_shortcode', 11 ); // Runs after wpautop(); note that $post global will be null when shortcodes run.
+add_filter( 'widget_text_content', 'wp_filter_content_tags', 12 ); // Runs after do_shortcode().
 
 add_filter( 'widget_block_content', 'do_blocks', 9 );
-add_filter( 'widget_block_content', 'wp_filter_content_tags' );
 add_filter( 'widget_block_content', 'do_shortcode', 11 );
+add_filter( 'widget_block_content', 'wp_filter_content_tags', 12 ); // Runs after do_shortcode().
 
 add_filter( 'block_type_metadata', 'wp_migrate_old_typography_shape' );
@@ -626 +627 @@
 add_action( 'template_redirect', 'wp_redirect_admin_locations', 1000 );
 
-// Shortcodes.
-add_filter( 'the_content', 'do_shortcode', 11 ); // AFTER wpautop().
-
 // Media.
 add_action( 'wp_playlist_scripts', 'wp_playlist_scripts' );

trunk/src/wp-includes/formatting.php

@@ -3981 +3981 @@
          * is wasteful and can lead to bugs in the image counting logic.
          */
-        $filter_image_removed = remove_filter( 'the_content', 'wp_filter_content_tags' );
+        $filter_image_removed = remove_filter( 'the_content', 'wp_filter_content_tags', 12 );
 
         /*
@@ -4004 +4004 @@
          */
         if ( $filter_image_removed ) {
-            add_filter( 'the_content', 'wp_filter_content_tags' );
+            add_filter( 'the_content', 'wp_filter_content_tags', 12 );
         }
 

trunk/src/wp-includes/media.php

@@ -5650 +5650 @@
 
     /*
-     * Skip programmatically created images within post content as they need to be handled together with the other
-     * images within the post content.
+     * Skip programmatically created images within content blobs as they need to be handled together with the other
+     * images within the post content or widget content.
      * Without this clause, they would already be considered within their own context which skews the image count and
      * can result in the first post content image being lazy-loaded or an image further down the page being marked as a
      * high priority.
      */
-    // TODO: Handle shortcode images together with the content (see https://core.trac.wordpress.org/ticket/58853).
-    if ( 'the_content' !== $context && 'do_shortcode' !== $context && doing_filter( 'the_content' ) ) {
+    if (
+        'the_content' !== $context && doing_filter( 'the_content' ) ||
+        'widget_text_content' !== $context && doing_filter( 'widget_text_content' ) ||
+        'widget_block_content' !== $context && doing_filter( 'widget_block_content' )
+    ) {
         /** This filter is documented in wp-includes/media.php */
         return apply_filters( 'wp_get_loading_optimization_attributes', $loading_attrs, $tag_name, $attr, $context );
+
     }
 

trunk/tests/phpunit/tests/formatting/wpTrimExcerpt.php

@@ -130 +130 @@
         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()' );
+        $this->assertSame( 12, has_filter( 'the_content', 'wp_filter_content_tags' ), 'wp_filter_content_tags() was not restored in wp_trim_excerpt()' );
     }
 
@@ -142 +142 @@
 
         // Remove wp_filter_content_tags() from 'the_content' filter generally.
-        remove_filter( 'the_content', 'wp_filter_content_tags' );
+        remove_filter( 'the_content', 'wp_filter_content_tags', 12 );
 
         wp_trim_excerpt( '', $post );

trunk/tests/phpunit/tests/media.php

@@ -4959 +4959 @@
     }
 
+    /**
+     * Tests that wp_filter_content_tags() and more specifically wp_get_loading_optimization_attributes() correctly
+     * handle shortcodes images together with the content that it is part of.
+     *
+     * Images within shortcodes as part of the content should be ignored by wp_get_loading_optimization_attributes() to
+     * avoid double processing. They should instead only be processed together with any other images as part of the
+     * content, to correctly count the original sequencing of those images.
+     *
+     * @ticket 58853
+     *
+     * @covers ::wp_filter_content_tags
+     * @covers ::wp_get_loading_optimization_attributes
+     */
+    public function test_wp_filter_content_tags_handles_shortcode_image_together_with_the_content() {
+        global $wp_query, $wp_the_query;
+
+        // Add shortcode that prints a large image, and a block type that wraps it.
+        add_shortcode(
+            'full_image',
+            static function ( $atts ) {
+                $atts = shortcode_atts(
+                    array(
+                        'id' => 0,
+                    ),
+                    $atts,
+                    'full_image'
+                );
+                return wp_get_attachment_image( (int) $atts['id'], 'full' );
+            }
+        );
+
+        /*
+         * Even though `do_shortcode()` runs before `wp_filter_content_tags()`, the image from the shortcode should not
+         * receive any loading optimization attributes because it needs to be considered together with the rest of the
+         * post content, within `wp_filter_content_tags()`.
+         * Since the hard-coded image appears before the shortcode image, it should receive `fetchpriority="high"`,
+         * despite the shortcode image being parsed before it.
+         */
+        $post_content  = '<img src="example.jpg" width="800" height="600">' . "\n";
+        $post_content .= '[full_image id="' . self::$large_id . '"]';
+        $post_content  = wpautop( $post_content );
+
+        /*
+         * Prepare the expected output:
+         * 1. On the first image (hard-coded in the content), expect `fetchpriority="high"`.
+         * 2. Replace the shortcode with its expected output, i.e. the full image. Expect neither
+         * `fetchpriority="high"` nor `loading="lazy"`.
+         */
+        $expected_content = $post_content;
+        $expected_content = str_replace(
+            '<img src="example.jpg"',
+            '<img fetchpriority="high" decoding="async" src="example.jpg"',
+            $expected_content
+        );
+        $expected_content = str_replace(
+            '[full_image id="' . self::$large_id . '"]',
+            str_replace(
+                '<img ',
+                '<img decoding="async" ',
+                wp_get_attachment_image(
+                    self::$large_id,
+                    'full',
+                    false,
+                    array(
+                        'decoding'      => false,
+                        'fetchpriority' => false,
+                        'loading'       => false,
+                    )
+                )
+            ),
+            $expected_content
+        );
+
+        // Create post with the content.
+        $post_id = self::factory()->post->create(
+            array(
+                'post_content' => $post_content,
+                'post_excerpt' => '',
+            )
+        );
+
+        // We have to run a main query loop so that the first 'the_content' context images are not lazy-loaded.
+        $wp_query     = new WP_Query( array( 'post__in' => array( $post_id ) ) );
+        $wp_the_query = $wp_query;
+
+        $content = '';
+        while ( have_posts() ) {
+            the_post();
+            $content = get_echo( 'the_content' );
+        }
+
+        // Cleanup.
+        remove_shortcode( 'full_image' );
+
+        $this->assertSame( $expected_content, $content );
+    }
+
+    /**
+     * Tests that wp_filter_content_tags() and more specifically wp_get_loading_optimization_attributes() correctly
+     * handle shortcodes images within the content, including within a block.
+     *
+     * Images within shortcodes as part of the content should be ignored by wp_get_loading_optimization_attributes() to
+     * avoid double processing. They should instead only be processed together with any other images as part of the
+     * content, to correctly count the original sequencing of those images.
+     *
+     * @ticket 58853
+     *
+     * @covers ::wp_filter_content_tags
+     * @covers ::wp_get_loading_optimization_attributes
+     */
+    public function test_wp_filter_content_tags_handles_shortcode_images_also_in_blocks_within_the_content() {
+        global $wp_query, $wp_the_query;
+
+        // Disable addition of `decoding="async"` as it is irrelevant for this test.
+        add_filter(
+            'wp_get_loading_optimization_attributes',
+            static function ( $loading_attrs ) {
+                if ( isset( $loading_attrs['decoding'] ) ) {
+                    unset( $loading_attrs['decoding'] );
+                }
+                return $loading_attrs;
+            }
+        );
+
+        // Add shortcode that prints a large image, and a block type that wraps it.
+        add_shortcode(
+            'full_image',
+            static function ( $atts ) {
+                $atts = shortcode_atts(
+                    array(
+                        'id' => 0,
+                    ),
+                    $atts,
+                    'full_image'
+                );
+                return wp_get_attachment_image( (int) $atts['id'], 'full' );
+            }
+        );
+        register_block_type(
+            'core/full-image-shortcode',
+            array(
+                'render_callback' => static function ( $atts ) {
+                    if ( empty( $atts['id'] ) ) {
+                        return '';
+                    }
+                    return do_shortcode( '[full_image id="' . $atts['id'] . '"]' );
+                },
+            )
+        );
+
+        /*
+         * Include the following images:
+         * 1. Using gallery shortcode. Expected `fetchpriority="high"`.
+         * 2. Regular hard-coded image.
+         * 3. Using custom shortcode within block.
+         * 4. Regular hard-coded image. Expected `loading="lazy"`.
+         *
+         * The first image is expected to be prioritized because it is the first (large enough) content image.
+         * The first three images are expected to not have lazy-loading because that is the default threshold for
+         * omitting the attribute.
+         * The fourth image is expected to be lazy-loaded as it is past the default threshold.
+         *
+         * The results will only be correct if all images are considered together. For example:
+         * * If the image within the shortcode would only be parsed after the rest of the content, it would miss the
+         * `fetchpriority="high"` attribute and instead incorrectly receive `loading="lazy"`. The second image would as
+         * a result incorrectly receive `fetchpriority="high"`.
+         * * If the image within the block would be parsed before the rest of the content, it would incorrectly receive
+         * the `fetchpriority="high"` attribute. Then the first image would no longer receive the attribute.
+         *
+         * To ensure that this works:
+         * * `wp_filter_content_tags()` must run after `do_blocks()` and `do_shortcode()`.
+         * * `wp_get_loading_optimization_attributes()` must bail early if any images from the content blob are being
+         * considered under a different context name than 'the_content'.
+         */
+        $post_content  = '[gallery ids="' . self::$large_id . '" size="large"]' . "\n";
+        $post_content .= '<img src="example.jpg" width="800" height="600">' . "\n";
+        $post_content .= '<p>Some text.</p>' . "\n";
+        $post_content .= '<!-- wp:core/full-image-shortcode {"id":' . self::$large_id . '} --><!-- /wp:core/full-image-shortcode -->' . "\n";
+        $post_content .= '<img src="example2.jpg" width="800" height="600">';
+
+        $post_id = self::factory()->post->create(
+            array(
+                'post_content' => $post_content,
+                'post_excerpt' => '',
+            )
+        );
+
+        /*
+         * Prepare the expected output:
+         * 1. Replace the shortcode with its expected output (ID increased by 1 because of static variable within
+         * the gallery_shortcode() function). Expect `fetchpriority="high"`, but not `loading="lazy"`.
+         * 2. Do not modify the second image as it is hard-coded in the content and expected to be unchanged.
+         * 3. Replace the block with its expected output, i.e. the full image from the shortcode within. Expect neither
+         * `fetchpriority="high"` nor `loading="lazy"`.
+         * 4. On the fourth image (hard-coded in the content), expect `loading="lazy"`.
+         */
+        $expected_content = $post_content;
+        $expected_content = str_replace(
+            '[gallery ids="' . self::$large_id . '" size="large"]',
+            str_replace(
+                array( ' loading="lazy"', '<img ' ),
+                array( '', '<img fetchpriority="high" ' ),
+                preg_replace_callback(
+                    '/gallery-(\d+)/',
+                    static function ( $match ) {
+                        return 'gallery-' . ( (int) $match[1] + 1 );
+                    },
+                    do_shortcode( '[gallery ids="' . self::$large_id . '" size="large" id="' . $post_id . '"]' )
+                )
+            ),
+            $expected_content
+        );
+        $expected_content = str_replace(
+            '<!-- wp:core/full-image-shortcode {"id":' . self::$large_id . '} --><!-- /wp:core/full-image-shortcode -->',
+            wp_get_attachment_image(
+                self::$large_id,
+                'full',
+                false,
+                array(
+                    'fetchpriority' => false,
+                    'loading'       => false,
+                )
+            ),
+            $expected_content
+        );
+        $expected_content = str_replace(
+            '<img src="example2.jpg"',
+            '<img loading="lazy" src="example2.jpg"',
+            $expected_content
+        );
+
+        // We have to run a main query loop so that the first 'the_content' context images are not lazy-loaded.
+        $wp_query     = new WP_Query( array( 'post__in' => array( $post_id ) ) );
+        $wp_the_query = $wp_query;
+
+        $content = '';
+        while ( have_posts() ) {
+            the_post();
+            $content = get_echo( 'the_content' );
+        }
+
+        // Cleanup.
+        remove_shortcode( 'full_image' );
+        unregister_block_type( 'core/full-image-shortcode' );
+
+        $this->assertSame( $expected_content, $content );
+    }
+
     private function reset_content_media_count() {
         // Get current value without increasing.
@@ -5319 +5567 @@
 
     /**
+     * Tests that the `do_shortcode` context results in a lazy-loaded image by default.
+     *
      * @ticket 58681
-     *
-     * @dataProvider data_wp_get_loading_optimization_attributes_in_shortcodes
-     */
-    public function test_wp_get_loading_optimization_attributes_in_shortcodes( $setup, $expected, $message ) {
+     * @ticket 58853
+     *
+     * @covers ::wp_get_loading_optimization_attributes
+     */
+    public function test_wp_get_loading_optimization_attributes_in_shortcodes() {
         $attr = $this->get_width_height_for_high_priority();
-        $setup();
-
-        // The first image processed in a shortcode should have fetchpriority set to high.
+
+        // Shortcodes processed outside of content blobs like 'the_content' always get `loading="lazy"`.
         $this->assertSameSetsWithIndex(
-            $expected,
+            array(
+                'decoding' => 'async',
+                'loading'  => 'lazy',
+            ),
             wp_get_loading_optimization_attributes( 'img', $attr, 'do_shortcode' ),
-            $message
-        );
-    }
-
-    public function data_wp_get_loading_optimization_attributes_in_shortcodes() {
-        return array(
-            'main_shortcode_image_should_have_fetchpriority_high'  => array(
-                'setup'    => function () {
-                    global $wp_query;
-
-                    // Set WP_Query to be in the loop and the main query.
-                    $wp_query->in_the_loop = true;
-                    $this->set_main_query( $wp_query );
-                },
-                'expected' => array(
-                    'decoding'      => 'async',
-                    'fetchpriority' => 'high',
-                ),
-                'message'  => 'Fetch priority not applied to during shortcode rendering.',
-            ),
-            'main_shortcode_image_after_threshold_is_loading_lazy' => array(
-                'setup'    => function () {
-                    global $wp_query;
-
-                    // Set WP_Query to be in the loop and the main query.
-                    $wp_query->in_the_loop = true;
-                    $this->set_main_query( $wp_query );
-
-                    // Set internal flags so lazy should be applied.
-                    wp_high_priority_element_flag( false );
-                    wp_increase_content_media_count( 3 );
-                },
-                'expected' => array(
-                    'decoding' => 'async',
-                    'loading'  => 'lazy',
-                ),
-                'message'  => 'Lazy-loading or decoding not applied to during shortcode rendering.',
-            ),
-            'shortcode_image_outside_of_the_loop_are_loaded_lazy'  => array(
-                'setup'    => function () {
-                    // Avoid setting up the WP_Query object for the loop.
-                    return;
-                },
-                'expected' => array(
-                    'decoding' => 'async',
-                    'loading'  => 'lazy',
-                ),
-                'message'  => 'Lazy-loading or decoding not applied to shortcodes outside the loop.',
-            ),
+            'Lazy-loading not applied to shortcodes outside the loop.'
+        );
+    }
+
+    /**
+     * Tests that the `do_shortcode` context does not result in loading optimization changes when used within a content
+     * blob.
+     *
+     * @ticket 58853
+     *
+     * @covers ::wp_get_loading_optimization_attributes
+     *
+     * @dataProvider data_get_filters_with_do_shortcode_callback
+     *
+     * @param string $filter_name The name of the filter to hook.
+     */
+    public function test_wp_get_loading_optimization_attributes_in_shortcodes_within_content_blob( $filter_name ) {
+        $result = null;
+
+        remove_all_filters( $filter_name );
+        add_filter(
+            $filter_name,
+            function ( $content ) use ( &$result ) {
+                $attr   = $this->get_width_height_for_high_priority();
+                $result = wp_get_loading_optimization_attributes( 'img', $attr, 'do_shortcode' );
+                return $content;
+            }
+        );
+        apply_filters( $filter_name, '' );
+
+        // Shortcodes processed within content blobs like 'the_content' should never get any loading optimization attributes.
+        $this->assertSame(
+            array(),
+            $result,
+            'Loading optimization unexpectedly applied to shortcodes within content blob.'
+        );
+    }
+
+    /**
+     * Gets filters for content blobs that by default have a `do_shortcode()` callback.
+     *
+     * @return array[]
+     */
+    public function data_get_filters_with_do_shortcode_callback() {
+        return self::text_array_to_dataprovider(
+            array(
+                'the_content',
+                'widget_text_content',
+                'widget_block_content',
+            )
         );
     }