The `wp_content_img_tag` filter can appear to run twice when used to wrap image tags

[peterwilsoncc]

Follow up to [53028] for #55347.

The new wp_content_img_tag can be problematic if a developer wishes to replace an <img /> tag by wrapping it in another tag, eg <span><img /><span> or, more likely, the <picture /> tag.

If an image tag appears twice or more in a posts content and none of the WP default changes (width and height, responsive images, lazy loading) then each time the filter runs the image tag will be replaced.

The same issue will occur if wp_filter_content_tags() is run multiple times on the same blob of HTML, although in that instance the double wrapping will occur for all images.

cc @pbearne

Please see example unit tests in the follow up comment.

[peterwilsoncc]

Three unit tests, two with the same image included twice under certain circumstances, one with the wp_filter_content_tags() function running on the content multiple times.

Please check there is not a situation in WP in which the wp_filter_content_tags() function will run twice on the same blob of content. I do wonder if any sites run it on save to avoid the performance hit, but I'd expect them to disable it on render.

<?php
/**
 * Test an image included twice that already contains the attributes WP adds.
 *
 * @ticket 55347
 */
public function test_wp_filter_content_tags_with_identical_images_custom_attributes() {
        $img         = get_image_tag( self::$large_id, '', '', '', 'medium' );
        $img         = str_replace( '<img ', '<img srcset="custom" sizes="custom" loading="custom" ', $img );
        $the_content = "$img\n\n$img";

        add_filter(
                'wp_content_img_tag',
                function( $filtered_image ) {
                        return "<span>$filtered_image</span>";
                }
        );

        $actual = wp_filter_content_tags( $the_content );
        $this->assertStringNotContainsString( '<span><span>', $actual );
}

/**
 * Test an image included twice when attribute modification is disabled.
 *
 * @ticket 55347
 */
public function test_wp_filter_content_tags_with_identical_images_disabled_core_filters() {
        $img         = get_image_tag( self::$large_id, '', '', '', 'medium' );
        $the_content = "$img\n\n$img";

        add_filter( 'wp_lazy_loading_enabled', '__return_false' );
        add_filter( 'wp_img_tag_add_width_and_height_attr', '__return_false' );
        add_filter( 'wp_img_tag_add_srcset_and_sizes_attr', '__return_false' );

        add_filter(
                'wp_content_img_tag',
                function( $filtered_image ) {
                        return "<span>$filtered_image</span>";
                }
        );

        $actual = wp_filter_content_tags( $the_content );
        $this->assertStringNotContainsString( '<span><span>', $actual );
}

/**
 * Test wrapping an image when the `wp_filter_content_tags()` function runs multiple times.
 *
 * @ticket 55347
 */
public function test_wp_filter_content_tags_with_the_function_run_twice() {
        $the_content = get_image_tag( self::$large_id, '', '', '', 'medium' );

        add_filter(
                'wp_content_img_tag',
                function( $filtered_image ) {
                        return "<span>$filtered_image</span>";
                }
        );

        $actual = wp_filter_content_tags( wp_filter_content_tags( $the_content ) );
        $this->assertStringNotContainsString( '<span><span>', $actual );
}

[pbearne]

This implies to me that the function is being run twice
This is got to be a performance hit

[flixos90]

@peterwilsoncc @pbearne The problem indeed occurs when the exact same image tag is included in the content multiple times.

This has not been a problem for WordPress core itself, since it will adjust the "duplicate" image tag in the same way. The following str_replace() (which by definition replaces all occurrences in the content) will technically not do anything since the unmodified image tag has already been replaced throughout the content before.

When wrapping an image tag without further modifying it though, it means that even after the replacement the original image tag remains in the content, hence the duplicate filter run replaces it again (which in case of a filter that wants to wrap the image with <span><img /></span> will get <span><span><img /></span></span>.

So while this is now a bug, it technically has always been a somewhat wasteful implementation, re-running the logic unnecessarily for duplicate instances of the same image tag. Fortunately there's an easy fix for it: We can simply unset the respective lookup key in the $images variable within wp_filter_content_tags() to not process the same tag again.

Note that we should make the same fix for iframes. They don't have a filter so this is technically not a bug there, but still just as wasteful.

[flixos90]

@peterwilsoncc

The same issue will occur if wp_filter_content_tags() is run multiple times on the same blob of HTML

While I definitely think we need to fix the duplicate logic execution here that causes the bug, calling wp_filter_content_tags() twice is a different topic. This is not something that should be done, WP core doesn't do it, and we also expect other the_content filters for example to not be run twice.

I'd be in favor of fixing the underlying issue of duplicate code execution within a single call to wp_filter_content_tags() since that is really the problem here. Whereas calling wp_filter_content_tags() twice on the same blob of content would be _doing_it_wrong :)

• Unsets lookup key for images and iframes after processing one, so that the respective image/iframe doesn't get processed again.
• Depending on how the filters are used, this is at least a performance enhancement, but can also fix a bug where a filter would unexpectedly modify a tag multiple times.

Trac ticket: https://core.trac.wordpress.org/ticket/55510

[flixos90]

@peterwilsoncc @pbearne I've created the above PR to fix this, ready for your review.

[peterwilsoncc]

Thanks @flixos90, I'll review the PR with a view to committing this over the next few hours.

While I definitely think we need to fix the duplicate logic execution here that causes the bug, calling wp_filter_content_tags() twice is a different topic. This is not something that should be done, WP core doesn't do it, and we also expect other the_content filters for example to not be run twice.

Agreed.

[peterwilsoncc]

In 53149:

Media: Run the wp_content_img_tag filter once per image.

Prevent multiple identical img tags in a block of content causing the wp_content_img_tag filter to fire multiple times for that image.

Follow up to [53028].

Props superpoincare, flixos90, pbearne, peterwilsoncc.
Fixes #55510.
See #55347.

Merged in https://core.trac.wordpress.org/changeset/53149, thanks.