Make WordPress Core

Opened 2 years ago

Closed 18 months ago

Last modified 17 months ago

#56588 closed defect (bug) (fixed)

WordPress puts "loading=lazy" on first image on archive page - wp_get_loading_attr_default bug

Reported by: salvoaranzulla's profile salvoaranzulla Owned by: flixos90's profile flixos90
Milestone: 6.3 Priority: high
Severity: normal Version: 5.9
Component: Media Keywords: has-patch, has-unit-tests, has-testing-info, has-dev-note
Focuses: performance Cc:

Description

If I see the HTML code of my archive page https://www.aranzulla.it/internet, I see that the first image has "loading=lazy" on the main loop, but it should not have it:

<img width="285" height="200" src="https://www.aranzulla.it/wp-content/contenuti/2017/02/ffcf69b7-285x200.jpg" class="attachment-home-bottom size-home-bottom wp-post-image" alt="Come controllare PEC" loading="lazy" />

WordPress doesn't put the loading=lazy on the first image using "wp_get_loading_attr_default" in wp-includes/media.php:

<?php
        // Increase the counter since this is a main query content element.
        $content_media_count = wp_increase_content_media_count();

        // If the count so far is below the threshold, return `false` so that the `loading` attribute is omitted.
        if ( $content_media_count <= wp_omit_loading_attr_threshold() ) {
                return false;
        }

There is a bug in wp_get_loading_attr_default. If you use the_excerpt(), the $content_media_count is increased of the number of images inside the article.

In my archive page, I have:

			<header>
				<a href="<?php the_permalink(); ?>"><span class="heading2"><?php the_title(); ?></span></a>
			</header>
			<p class="column"><?php the_excerpt(); ?></p>
			<figure class="column">
				<?php the_post_thumbnail('home-bottom', array( 'alt' => get_the_title())); ?>
			</figure>

If the first article has 4 photos (that doesn't show in the_excerpt()), the the_post_thumbnail starts from number 5 and WordPress adds "loading=lazy".

To reproduce the bug, you can:

1) Edit media.php adding a print_r($content_media_count);:

<?php
        // Increase the counter since this is a main query content element.
        $content_media_count = wp_increase_content_media_count();

        print_r($content_media_count); // added to see the counter

        // If the count so far is below the threshold, return `false` so that the `loading` attribute is omitted.
        if ( $content_media_count <= wp_omit_loading_attr_threshold() ) {
                return false;
        }

2) Create an archive page/homepage where the_excerpt + thumbnail are used and with few images in that article.

3) When WordPress arrives to the thumbnail, the counter is not 1 even if it's the first image.

Attachments (2)

Schermata 2022-09-16 alle 14.35.05.2.png (549.7 KB) - added by salvoaranzulla 2 years ago.
The number shows the counter of WordPress. The first article has 3 images inside.
Screenshot 2023-04-06 alle 08.41.46.png (431.0 KB) - added by salvoaranzulla 20 months ago.

Download all attachments as: .zip

Change History (30)

@salvoaranzulla
2 years ago

The number shows the counter of WordPress. The first article has 3 images inside.

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


2 years ago

#2 @antpb
2 years ago

  • Milestone changed from Awaiting Review to 6.2

Thanks for the ticket @salvoaranzulla ! It looks like some work toward fixing this was landed in 5.9: https://core.trac.wordpress.org/ticket/53675

It is possible this edge case was not identified in the scope of that ticket. Moving to 6.2 to investigate.

cc @flixos90

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


22 months ago

#4 @joemcgill
22 months ago

  • Focuses performance added

Related to: #53675, #56930

#5 @flixos90
22 months ago

  • Keywords reporter-feedback added

Thanks @salvoaranzulla, I have this ticket on my radar and am planning a coordinated effort to identify the remaining lazy-loading problems including the one you're having.

I have tested the scenario you're describing. Unfortunately, it looks like the excerpt is not the cause for this problem. I am using a post with a few images in it, and I display the_excerpt() before calling the_post_thumbnail(), and my post thumbnail, as expected, does not have the loading="lazy" attribute.

I certainly acknowledge though that there must be another bug happening that is causing that problem on your site. Can you maybe provide a bit more context? If at all possible, would you mind sharing a larger code sample of the template used on that page? I would like to figure out what the exact bug is.

#6 @flixos90
22 months ago

  • Milestone changed from 6.2 to Future Release
  • Owner set to flixos90
  • Status changed from new to assigned
  • Version changed from 6.0.2 to 5.9

@antpb @joemcgill I am working on a coordinated effort to identify the primary remaining issues with LCP images being lazy-loaded, and this ticket certainly will lead to one of them.

Unfortunately, per my above comment, the suggested problem does not appear to be the actual bug here, so this will require more time for investigation. Unless we can replicate this bug and identify a simple solution, we should work on this for 6.3.

I will move this to "Future Release" for now, but I have (and had) already taken a note to prioritize this early in the 6.3 cycle.

#7 @flixos90
21 months ago

  • Milestone changed from Future Release to 6.3

#8 @mukesh27
20 months ago

  • Keywords needs-patch added

#9 @flixos90
20 months ago

  • Keywords needs-patch removed

@salvoaranzulla Can you please provide more detail on how the excerpts are rendered on the site where this bug occurs?

A few more specific questions:

  • Are you using blocks for the excerpt?
  • Does some part of the theme or a plugin on your site call the_content() or get_the_content() for each of these posts?

As mentioned above, I have been unable to replicate this bug without additional detail. The more information you can share on this, the better.

#10 @salvoaranzulla
20 months ago

Hi,

Here is an update with latest WordPress version (6.2). I confirm that the bug is also in the last version of WordPress.

1) On wp-includes/media.php I have added:

<?php

print_r($content_media_count); // added to see the counter

after

<?php
// Increase the counter since this is a main query content element.
$content_media_count = wp_increase_content_media_count();


On my archive page, I see:

https://core.trac.wordpress.org/attachment/ticket/56588/Screenshot%202023-04-06%20alle%2008.41.46.png

2) My archive code page is the following:

		<?php 
			$p = 0; 
			$i = 0;	
				while ( have_posts() ) : the_post(); 
				if($p == 0): 
			?>
		<article class="contentPreview">
			<header>
				<a href="<?php the_permalink(); ?>"><span class="heading2"><?php the_title(); ?></span></a>
			</header>
			<p class="column"><?php the_excerpt(); ?></p>
			<figure class="column">
				<?php the_post_thumbnail('home-bottom', array( 'alt' => get_the_title())) ?>
			</figure>
		</article>
		<div class="articleGrid">
			<?php 
				else: 
				if ($i % 2 == 0) echo '<div class="row">'; 
			?>     
				<article class="contentPreview column">
					<header>
						<a href="<?php the_permalink(); ?>"><span class="heading4"><?php the_title(); ?></span></a>
					</header>
					<p class="small"><?php the_excerpt(); ?></p>
				</article>
			<?php 
				if ($i % 2 != 0) echo '</div>';
				$i++;
			?>
		<?php endif;  $p++;  endwhile; ?>
		</div>

3) The "loading=lazy" has been added to the images because WordPress thinks that the image is the image number 10 (my post has 9 images inside).

#11 @flixos90
19 months ago

@salvoaranzulla Thank you for the additional detail, I think I have found the problem: WordPress core's excerpt function relies on the filter callback wp_trim_excerpt() which parses the full post content unless a manual excerpt text has been provided. This results in the images being counted when they shouldn't be.

Can you test adding the following code to your site (e.g. in the theme's functions.php)? If I'm correct, that should function as a workaround for the bug until we have fixed it in WordPress core itself.

add_filter(
	'get_the_excerpt',
	function( $excerpt ) {
		remove_filter( 'the_content', 'wp_filter_content_tags' );
		return $excerpt;
	},
	9
);
add_filter(
	'get_the_excerpt',
	function( $excerpt ) {
		add_filter( 'the_content', 'wp_filter_content_tags' );
		return $excerpt;
	},
	11
);

#12 @flixos90
19 months ago

  • Priority changed from normal to high

Addressing the remaining lazy-loading related performance issues is a high priority for the performance team for the 6.3 cycle.

#13 @salvoaranzulla
19 months ago

Hi,

I confirm that your code in functions.php has solved the problem:

<?php
add_filter(
        'get_the_excerpt',
        function( $excerpt ) {
                remove_filter( 'the_content', 'wp_filter_content_tags' );
                return $excerpt;
        },
        9
);
add_filter(
        'get_the_excerpt',
        function( $excerpt ) {
                add_filter( 'the_content', 'wp_filter_content_tags' );
                return $excerpt;
        },
        11
);

This ticket was mentioned in PR #4439 on WordPress/wordpress-develop by @flixos90.


19 months ago
#14

  • Keywords has-patch has-unit-tests added

This PR temporarily unhooks wp_filter_content_tags() from the_content filter when invoking it in wp_trim_excerpt().

  • Since that function only gets the content to generate the excerpt, which never includes any HTML tags, calling wp_filter_content_tags() is wasteful and unnecessary in that context.
  • More importantly, calling it there leads to bugs like the one outlined in the Trac ticket.

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

#15 @flixos90
19 months ago

  • Keywords reporter-feedback removed

@mukesh27 commented on PR #4439:


19 months ago
#16

@felixarntz, I tried to reproduce the issue, but I couldn't. Could you please provide the steps to reproduce the issue?

@flixos90 commented on PR #4439:


19 months ago
#17

@mukeshpanchal27 In order to test this, you need to use a theme that on its archive pages (e.g. the blog) shows post excerpts (the_excerpt()) and post featured images (the_post_thumbnail()).

Assuming that the theme displays the featured image first, and then the excerpt, for each post, you'll need to do the following:

  1. Create a post that has a featured image.
  2. Create another post (i.e. newer than the first post) that doesn't have a featured image, but has 3 images within its post content.
  3. View the blog, where the 2. post should show up first, and the 1. post should show up second, since it's older. The only image you should see is the featured image of the one post, since excerpts are always plain text and don't include images.
  4. Without this PR, you should notice that the featured image has loading="lazy" on it, which is incorrect since it's the first image on the blog. With this PR, the featured image shouldn't have loading="lazy" anymore, which would be correct.

@flixos90 commented on PR #4439:


18 months ago
#18

@mukeshpanchal27

I was unable to reproduce the same issue in Twenty Twenty-One and Twenty Nineteen.

Have you ensured you test with a theme and template that uses the_excerpt() and not the_content()?

  • For Twenty Twenty-One you'll need to make sure the setting is set to display excerpts.
  • For Twenty Nineteen, there is no such setting and it always uses the_content(), except in the search template. So in Twenty Nineteen, you would need to create 2 posts that satisfy a specific search query, and then use the search to validate that the bug is fixed.

#19 @costdev
18 months ago

Hi @flixos90, I can't seem to reproduce the original issue. Can you check my testing steps and let me know where I'm going wrong?

  1. Use Twenty-Twenty One.
  2. In Customizer, ensure the "Excerpt Settings" are set to "Summary".
  3. Create a post with a featured image, but no images in its content.
  4. Create another post with no featured image, but 3 images in its content.
  5. View the Categorized category page.
  6. Inspect the featured image of the original post, which should be the only image on the page.
  7. It should have loading="lazy" ❌ It doesn't.

I tested on 5.9 which this issue was originally reported against, but couldn't reproduce it. Where am I going wrong? If the above steps seem to be accurate, can you try reproducing the issue again on 5.9?

#20 @flixos90
18 months ago

@costdev Thanks for the feedback. I took a closer look, and indeed the instructions weren't entirely correct. Since blocks are removed from the content for the excerpt anyway, this problem is a bit more specific, and it applies only to images that are not part of a block. So this could be either legacy content not created with Gutenberg, or content injected through a plugin, e.g. a page builder or a plugin that adds some markup to each post.

So the 4th step needs to be clarified to:
"Create another post with no featured image, but 3 images in its content. All of these images should appear outside of blocks."

An easy way to enforce this behavior even when using the block editor would be to add the images as core/image blocks as usual, then go into the overall "Code Editor" and remove all the block comments for those core/image blocks so that the editor interprets it as "Classic Editor" markup.

Can you please give it a retry like that? That should see the original issue and it being fixed by the PR.

@flixos90 commented on PR #4439:


18 months ago
#21

@mukeshpanchal27 I took a closer look at the test instructions again, see https://core.trac.wordpress.org/ticket/56588#comment:20 for the main clarification.

#22 @costdev
18 months ago

Thanks for the updated test instructions @flixos90!

Test Report

Patch tested: https://patch-diff.githubusercontent.com/raw/WordPress/wordpress-develop/pull/4439.diff

Steps to Reproduce or Test

  1. Create a post with a featured image.
  2. Create another post with no featured image, but insert a Classic Block and add 3 images inside it.
  3. View the category page. You should only see the featured image.
  4. 🐞 Inspect the featured image.

Expected Results

When reproducing a bug:

  • ❌ The featured image should have loading="lazy".

When testing a patch to validate it works as expected:

  • ✅ The featured image should not have loading="lazy".

Environment

  • WordPress: 6.3-alpha-55505
  • PHP: 7.4.33
  • Server: Apache/2.4.56 (Ubuntu)
  • Database: mysqli (Server: 5.7.41-0ubuntu0.18.04.1 / Client: mysqlnd 7.4.33)
  • Browser: Chrome 113.0.0.0 (Windows 10/11)
  • Theme: Twenty Nineteen, Twenty-Twenty One, Twenty-Twenty Two, Twenty-Twenty Three
  • MU-Plugins: None activated
  • Plugins: None activated

Actual Results

When reproducing a bug/defect:

  • ❌ The featured image had loading="lazy". Issue reproduced on all tested themes.

When testing the bugfix patch:

  • ✅ The featured image did not have loading="lazy". Issue resolved with the patch on all tested themes.

#23 @costdev
18 months ago

  • Keywords has-testing-info added

#24 @flixos90
18 months ago

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

In 55850:

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.

#25 @flixos90
18 months ago

  • Keywords needs-dev-note added

#28 @flixos90
17 months ago

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