Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#55347 closed enhancement (fixed)

There should be filter for image tag in wp_filter_content_tags()

Reported by: pbearne's profile pbearne Owned by: flixos90's profile flixos90
Milestone: 6.0 Priority: normal
Severity: normal Version:
Component: Media Keywords: has-patch has-unit-tests needs-docs has-dev-note
Focuses: Cc:

Description


Change History (19)

#1 @pbearne
2 years ago

  • Summary changed from they should be filter for image tag in wp_filter_content_tags() to There should be filter for image tag in wp_filter_content_tags()

This ticket was mentioned in PR #2394 on WordPress/wordpress-develop by pbearne.


2 years ago
#2

  • Keywords has-patch has-unit-tests added

This patch adds a filter to wp_filter_content_tags() so we can add attributes to the img tags in content

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

#3 @peterwilsoncc
2 years ago

@pbearne Are you able to add some background to the description so the ticket can be properly considered, possibly for 6.0? My understanding is it's come from the performance team but additional information would help.

#4 @pbearne
2 years ago

Without this filter, you have to duplicate the regex code from WP core and create a function like this

<?php
        public function filter_content_tags( $content, $context = null ) {
                if ( null === $context ) {
                        $context = current_filter();
                }

                if ( ! preg_match_all( '/<(img|iframe)\s[^>]+>/', $content, $matches, PREG_SET_ORDER ) ) {
                        return $content;
                }

                // List of the unique `img` tags found in $content.
                $images = array();

                // List of the unique `iframe` tags found in $content.
                $iframes = array();

                foreach ( $matches as $match ) {
                        list( $tag, $tag_name ) = $match;

                        switch ( $tag_name ) {
                                case 'img':
                                        if ( preg_match( '/wp-image-([0-9]+)/i', $tag, $class_id ) ) {
                                                $attachment_id = absint( $class_id[1] );

                                                if ( $attachment_id ) {
                                                        // If exactly the same image tag is used more than once, overwrite it.
                                                        // All identical tags will be replaced later with 'str_replace()'.
                                                        $images[ $tag ] = $attachment_id;
                                                        break;
                                                }
                                        }
                                        $images[ $tag ] = 0;
                                        break;
                        }
                }

                // Reduce the array to unique attachment IDs.
                $attachment_ids = array_unique( array_filter( array_values( $images ) ) );

                if ( count( $attachment_ids ) > 1 ) {
                        /*
                         * Warm the object cache with post and meta information for all found
                         * images to avoid making individual database calls.
                         */
                        _prime_post_caches( $attachment_ids, false, true );
                }

                // Iterate through the matches in order of occurrence as it is relevant for whether or not to lazy-load.
                foreach ( $matches as $match ) {
                        // Filter an image match.
                        if ( isset( $images[ $match[0] ] ) ) {
                                $filtered_image = $match[0];
                                $attachment_id  = $images[ $match[0] ];
                                /**
                                 * Filters img tag that will be injected into the content.
                                 *
                                 * @param string $filtered_image the img tag with attributes being created that will
                                 *                                    replace the source img tag in the content.
                                 * @param string $context Optional. Additional context to pass to the filters.
                                 *                        Defaults to `current_filter()` when not set.
                                 * @param int $attachment_id the ID of the image attachment.
                                 *
                                 * @since 1.0.0
                                 *
                                 */
                                $filtered_image = apply_filters( 'wp_dominant_color_img_tag_add_adjust', $filtered_image, $context, $attachment_id );

                                if ( $filtered_image !== $match[0] ) {
                                        $content = str_replace( $match[0], $filtered_image, $content );
                                }
                        }
                }

                return $content;
        }

In this example, I have added a filtered in the same logical place as the requested change and I simple hook into it to add the additional tags to the images

which has to be maintained if core updates the regex it's unlikely that plugin authors might miss it

I hope this makes sense

Info: We are working on a patch to add an images dominant color placeholder to lazy loaded images

Paul


#5 @flixos90
2 years ago

  • Milestone changed from Awaiting Review to 6.0
  • Owner set to flixos90
  • Status changed from new to reviewing

This request has come up before in other places (e.g. media chat), and I agree it's worth addressing. Currently, every plugin that wants to do something to image tags in the frontend has to re-implement the same logic that core already has here.

We also ran into it during the Performance Lab plugin development, but it has come up before. I think this enhancement is worth being included in WordPress 6.0.

#6 @pbearne
2 years ago

PR refreshed with review changes

#7 @flixos90
2 years ago

  • Keywords commit added

This is good to go I think. I'm planning to commit it next week.

#8 @flixos90
2 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 53028:

Media: Introduce wp_content_img_tag filter.

This filter allows modifying individual img tags within a blob of content that are by default processed by the wp_filter_content_tags() function. The addition of this filter facilitates plugins that tweak images to accomplish this goal without re-implementing duplicate content image parser logic, which furthermore can have a negative performance impact due to additional regular expressions.

In addition to the filterable img tag, the filter receives the context (typically the function or filter in which the content is parsed) and the attachment ID. The latter may be 0, in case the image is not an attachment (for example when it is an external image URL).

Props adamsilverstein, flixos90, pbearne, peterwilsoncc.
Fixes #55347.

#10 follow-up: @superpoincare
2 years ago

This filter seems to be running twice.

So if I modify the img tag, like add some attribute, it will be added twice.

#11 in reply to: ↑ 10 @pbearne
2 years ago

Replying to superpoincare:

This filter seems to be running twice.

So if I modify the img tag, like add some attribute, it will be added twice.

Can you share the code that runs this twice?

#12 @superpoincare
2 years ago

Hi @pbearne,

Something like:

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

leads to:

<tag><tag><img src="https://demo2.cloudwp.dev/trial-3z1wywz1/wp-content/themes/twentytwentytwo/assets/images/flight-path-on-transparent-d.png" alt="Illustration of a bird flying."/></tag></tag>

in HTML.

#13 @pbearne
2 years ago

I added another unit test
I am not seeing this issue
So we need check the function is not being called twice

<?php
        public function test_wp_filter_content_tag_test() {
                $content = '<p>Image, standard. Should have srcset and sizes.</p>
                        <img src="http://url.tld" alt="" />';
                add_filter(
                        'wp_content_img_tag',
                        static function( $filtered_image ) {
                                return "<tag>$filtered_image</tag>";
                        }
                );
                $this->assertSame( '<p>Image, standard. Should have srcset and sizes.</p>
                        <tag><img src="http://url.tld" alt="" /></tag>', wp_filter_content_tags( $content ) );
        }
Last edited 2 years ago by pbearne (previous) (diff)

#14 @peterwilsoncc
2 years ago

I was able to reproduce the issue @superpoincare reports:

The issue occurs when an identical image tag appears in the_content twice and either of the following are true:

  • the image includes each of the attributes WP Core adds by default, prompting core not to replace them
  • the addition of these custom attributes is disabled via filters

WP has done this in the core functionality for a while, but it's never mattered as the <img /> tag was only replaced by an <img /> tag.

With the filter, a developer may wish to replace the images tag by wrapping it in another tag (for example <picture /> with an <img /> fallback). With the new filter, Core does need to replace the tag once and once only.

I'll create a follow up issue and assign it to @flixos90


These are the tests I used to reproduce the issue.

<?php
/**
 * @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 );
}

/**
 * @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 );
}

#15 @peterwilsoncc
2 years ago

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.

#16 @milana_cap
2 years ago

  • Keywords needs-dev-note needs-docs added

#18 @flixos90
2 years ago

@pbearne Thanks for writing this up! I've reviewed your dev note and left a few suggestions.

#19 @spacedmonkey
2 years ago

  • Keywords has-dev-note added; commit needs-dev-note removed
Note: See TracTickets for help on using tickets.