WordPress.org
Get WordPress

Make WordPress Core

Changeset 55850


Ignore:
Timestamp:
05/23/2023 06:23:59 PM (16 months ago)
Author:
flixos90
Message:

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.

Props costdev, flixos90, joemcgill, mukesh27, salvoaranzulla, spacedmonkey, thekt12, westonruter.
Fixes #56588.

Location:
trunk
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/wp-includes/formatting.php

    r55844 r55850  
    39383938        $text = excerpt_remove_blocks( $text );
    39393939
     3940        /*
     3941         * Temporarily unhook wp_filter_content_tags() since any tags
     3942         * within the excerpt are stripped out. Modifying the tags here
     3943         * is wasteful and can lead to bugs in the image counting logic.
     3944         */
     3945        $filter_removed = remove_filter( 'the_content', 'wp_filter_content_tags' );
     3946
    39403947        /** This filter is documented in wp-includes/post-template.php */
    39413948        $text = apply_filters( 'the_content', $text );
    39423949        $text = str_replace( ']]>', ']]>', $text );
     3950
     3951        /**
     3952         * Only restore the filter callback if it was removed above. The logic
     3953         * to unhook and restore only applies on the default priority of 10,
     3954         * which is generally used for the filter callback in WordPress core.
     3955         */
     3956        if ( $filter_removed ) {
     3957            add_filter( 'the_content', 'wp_filter_content_tags' );
     3958        }
    39433959
    39443960        /* translators: Maximum number of words used in a post excerpt. */

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

    r53562 r55850  
    9393        $this->assertSame( 'Post content', wp_trim_excerpt( false, $post ) );
    9494    }
     95
     96    /**
     97     * Tests that `wp_trim_excerpt()` unhooks `wp_filter_content_tags()` from 'the_content' filter.
     98     *
     99     * @ticket 56588
     100     */
     101    public function test_wp_trim_excerpt_unhooks_wp_filter_content_tags() {
     102        $post = self::factory()->post->create();
     103
     104        /*
     105         * Record that during 'the_content' filter run by wp_trim_excerpt() the
     106         * wp_filter_content_tags() callback is not used.
     107         */
     108        $has_filter = true;
     109        add_filter(
     110            'the_content',
     111            static function( $content ) use ( &$has_filter ) {
     112                $has_filter = has_filter( 'the_content', 'wp_filter_content_tags' );
     113                return $content;
     114            }
     115        );
     116
     117        wp_trim_excerpt( '', $post );
     118
     119        $this->assertFalse( $has_filter, 'wp_filter_content_tags() was not unhooked in wp_trim_excerpt()' );
     120    }
     121
     122    /**
     123     * Tests that `wp_trim_excerpt()` doesn't permanently unhook `wp_filter_content_tags()` from 'the_content' filter.
     124     *
     125     * @ticket 56588
     126     */
     127    public function test_wp_trim_excerpt_should_not_permanently_unhook_wp_filter_content_tags() {
     128        $post = self::factory()->post->create();
     129
     130        wp_trim_excerpt( '', $post );
     131
     132        $this->assertSame( 10, has_filter( 'the_content', 'wp_filter_content_tags' ), 'wp_filter_content_tags() was not restored in wp_trim_excerpt()' );
     133    }
     134
     135    /**
     136     * Tests that `wp_trim_excerpt()` doesn't restore `wp_filter_content_tags()` if it was previously unhooked.
     137     *
     138     * @ticket 56588
     139     */
     140    public function test_wp_trim_excerpt_does_not_restore_wp_filter_content_tags_if_previously_unhooked() {
     141        $post = self::factory()->post->create();
     142
     143        // Remove wp_filter_content_tags() from 'the_content' filter generally.
     144        remove_filter( 'the_content', 'wp_filter_content_tags' );
     145
     146        wp_trim_excerpt( '', $post );
     147
     148        // Assert that the filter callback was not restored after running 'the_content'.
     149        $this->assertFalse( has_filter( 'the_content', 'wp_filter_content_tags' ) );
     150    }
    95151}

  • trunk/tests/phpunit/tests/media.php

    r55847 r55850  
    7474        wp_delete_attachment( self::$large_id, true );
    7575        parent::tear_down_after_class();
     76    }
     77
     78    /**
     79     * Ensures that the static content media count and related filter are reset between tests.
     80     */
     81    public function set_up() {
     82        parent::set_up();
     83
     84        $this->reset_content_media_count();
     85        $this->reset_omit_loading_attr_filter();
    7686    }
    7787
    … …  
    35683578
    35693579        $query = $this->get_new_wp_query_for_published_post();
    3570         $this->reset_content_media_count();
    3571         $this->reset_omit_loading_attr_filter();
    35723580
    35733581        while ( have_posts() ) {
    … …  
    36143622        $query = $this->get_new_wp_query_for_published_post();
    36153623        $this->set_main_query( $query );
    3616         $this->reset_content_media_count();
    3617         $this->reset_omit_loading_attr_filter();
    36183624
    36193625        // Use the filter to alter the threshold for not lazy-loading to the first five elements.
    … …  
    36563662        $query = $this->get_new_wp_query_for_published_post();
    36573663        $this->set_main_query( $query );
    3658         $this->reset_content_media_count();
    3659         $this->reset_omit_loading_attr_filter();
    36603664
    36613665        while ( have_posts() ) {
    … …  
    37083712
    37093713        $wp_query = $this->get_new_wp_query_for_published_post();
    3710         $this->reset_content_media_count();
    3711         $this->reset_omit_loading_attr_filter();
    37123714
    37133715        do_action( 'get_header' );
    … …  
    37333735        $wp_query = $this->get_new_wp_query_for_published_post();
    37343736        $this->set_main_query( $wp_query );
    3735         $this->reset_content_media_count();
    3736         $this->reset_omit_loading_attr_filter();
    37373737
    37383738        // Lazy if header not called.
    … …  
    37563756        $wp_query = $this->get_new_wp_query_for_published_post();
    37573757        $this->set_main_query( $wp_query );
    3758         $this->reset_content_media_count();
    3759         $this->reset_omit_loading_attr_filter();
    37603758
    37613759        do_action( 'get_header' );
    … …  
    37793777        $wp_query = $this->get_new_wp_query_for_published_post();
    37803778        $this->set_main_query( $wp_query );
    3781         $this->reset_content_media_count();
    3782         $this->reset_omit_loading_attr_filter();
    37833779
    37843780        do_action( 'get_header' );
    … …  
    38063802        $wp_query = $this->get_new_wp_query_for_published_post();
    38073803        $this->set_main_query( $wp_query );
    3808         $this->reset_content_media_count();
    3809         $this->reset_omit_loading_attr_filter();
    38103804
    38113805        // Ensure header and footer is called.
    … …  
    38663860        $wp_the_query = $wp_query;
    38673861        $post         = get_post( self::$post_ids['publish'] );
    3868         $this->reset_content_media_count();
    3869         $this->reset_omit_loading_attr_filter();
    38703862
    38713863        $_wp_current_template_content = '<!-- wp:post-content /-->';
    … …  
    39233915        $wp_the_query = $wp_query;
    39243916        $post         = get_post( self::$post_ids['publish'] );
    3925         $this->reset_content_media_count();
    3926         $this->reset_omit_loading_attr_filter();
    39273917
    39283918        $_wp_current_template_content = '<!-- wp:post-featured-image /--> <!-- wp:post-content /-->';
    … …  
    40144004        $wp_query     = new WP_Query( array( 'post__in' => array( self::$post_ids['publish'] ) ) );
    40154005        $wp_the_query = $wp_query;
    4016         $this->reset_content_media_count();
    4017         $this->reset_omit_loading_attr_filter();
     4006
    40184007        $content = '';
    40194008        while ( have_posts() ) {
    … …  
    40774066            'wp_get_attachment_image' => array( 'context' => 'wp_get_attachment_image' ),
    40784067        );
     4068    }
     4069
     4070    /**
     4071     * Tests that the content media count is not affected by `the_excerpt()` calls for posts that contain images.
     4072     *
     4073     * @ticket 56588
     4074     *
     4075     * @covers ::wp_trim_excerpt
     4076     */
     4077    public function test_the_excerpt_does_not_affect_content_media_count() {
     4078        global $wp_query, $wp_the_query;
     4079
     4080        /*
     4081         * Use the filter to alter the threshold for not lazy-loading to the first 2 elements,
     4082         * then use a post that contains exactly 2 images.
     4083         */
     4084        $this->force_omit_loading_attr_threshold( 2 );
     4085        $post_content  = '<img src="example.jpg" width="800" height="600">';
     4086        $post_content .= '<p>Some text.</p>';
     4087        $post_content .= '<img src="example2.jpg" width="800" height="600">';
     4088
     4089        $post_id = self::factory()->post->create(
     4090            array(
     4091                'post_content' => $post_content,
     4092                'post_excerpt' => '',
     4093            )
     4094        );
     4095
     4096        $wp_query     = new WP_Query( array( 'post__in' => array( $post_id ) ) );
     4097        $wp_the_query = $wp_query;
     4098
     4099        while ( have_posts() ) {
     4100            the_post();
     4101
     4102            // Call `the_excerpt()` without generating output.
     4103            get_echo( 'the_excerpt' );
     4104        }
     4105
     4106        // The only way to access the value is by calling this function without increasing the value.
     4107        $content_media_count = wp_increase_content_media_count( 0 );
     4108
     4109        // Assert that the media count was not increased even though there are 3 images in the post's content.
     4110        $this->assertSame( 0, $content_media_count );
     4111    }
     4112
     4113    /**
     4114     * Tests that the lazy-loading result is not affected by `the_excerpt()` calls for posts that
     4115     * contain images.
     4116     *
     4117     * Printing the excerpt for a post that contains images in its content prior to its featured image should result in
     4118     * that featured image not being lazy-loaded, since the images in the post content aren't displayed in the excerpt.
     4119     *
     4120     * @ticket 56588
     4121     *
     4122     * @covers ::wp_trim_excerpt
     4123     */
     4124    public function test_the_excerpt_does_not_affect_omit_lazy_loading_logic() {
     4125        global $wp_query, $wp_the_query;
     4126
     4127        /*
     4128         * Use the filter to alter the threshold for not lazy-loading to the first 2 elements,
     4129         * then use a post that contains exactly 2 images.
     4130         */
     4131        $this->force_omit_loading_attr_threshold( 2 );
     4132        $post_content  = '<img src="example.jpg" width="800" height="600">';
     4133        $post_content .= '<p>Some text.</p>';
     4134        $post_content .= '<img src="example2.jpg" width="800" height="600">';
     4135
     4136        $post_id           = self::factory()->post->create(
     4137            array(
     4138                'post_content' => $post_content,
     4139                'post_excerpt' => '',
     4140            )
     4141        );
     4142        $featured_image_id = self::$large_id;
     4143        update_post_meta( $post_id, '_thumbnail_id', $featured_image_id );
     4144
     4145        $expected_image_tag = get_the_post_thumbnail( $post_id, 'post-thumbnail', array( 'loading' => false ) );
     4146
     4147        $wp_query     = new WP_Query( array( 'post__in' => array( $post_id ) ) );
     4148        $wp_the_query = $wp_query;
     4149
     4150        $output = '';
     4151        while ( have_posts() ) {
     4152            the_post();
     4153
     4154            // Print excerpt first, then the featured image.
     4155            $output .= get_echo( 'the_excerpt' );
     4156            $output .= get_echo( 'the_post_thumbnail' );
     4157        }
     4158
     4159        $this->assertStringContainsString( $expected_image_tag, $output );
    40794160    }
    40804161
Note: See TracChangeset for help on using the changeset viewer.