manually setting fetchpriority on image should prevent core from adding fetchpriority attribute

[adamsilverstein]

When working on testing the automated fetchpriorty attribute core now adds for images, I discovered this bug:

If users manually set fetchpriority="high" on an image either programmatically or using a plugin, core will not change that setting, however it unexpectedly sets fetchpriority="high" on another image.

Steps to reproduce

1. install this plugin to add a manual fetchpriority dropdown: ​https://github.com/adamsilverstein/wp-fetchpriority-control
2. create a post with several large images
3. select the first image and apply fetchpriority="high" (under advanced)
4. publish the post and view its source

Expected results:
Only the first image should have the fetchpriority="high" attribute, setting high fetchpriority on more than one image reduces the effect of adding the attribute.

Actual result:
Core applies fetchpriority="high" to the second image so the first two images contain the attribute.

[flixos90]

@adamsilverstein How does the plugin add fetchpriority="high"? Is it dynamic (via PHP logic), or is it hard-coded in the saved block content in the database? Would be great if you could provide additional context on that.

[adamsilverstein]

@adamsilverstein How does the plugin add fetchpriority="high"? Is it dynamic (via PHP logic), or is it hard-coded in the saved block content in the database? Would be great if you could provide additional context on that.

Good question!

It is saved as a block attribute, then applied to the markup dynamically using the render_block filter - see ​https://github.com/adamsilverstein/wp-fetchpriority-control/blob/dd0281b319794d7fe44317cb588a6c135f970200/wp-fetchpriority-control.php#L34-L54

Do you think the timing of this filtering is part of the issue?

[flixos90]

Replying to adamsilverstein:

It is saved as a block attribute, then applied to the markup dynamically using the render_block filter - see ​https://github.com/adamsilverstein/wp-fetchpriority-control/blob/dd0281b319794d7fe44317cb588a6c135f970200/wp-fetchpriority-control.php#L34-L54

Do you think the timing of this filtering is part of the issue?

Hmm, the render_block filter should be fired before the relevant logic. I wonder whether the issue is more about the image ordering. Here's why: WordPress core only goes through all the images sequentially. So by the time it encounters the image that your plugin has added fetchpriority="high" to, it may have already added the attribute to one of the images before.

When you encounter the bug, is the image that WordPress adds fetchpriority="high" to before the image that your plugin adds fetchpriority="high" too? If so, that would explain the problem.

[adamsilverstein]

Hmm, the render_block filter should be fired before the relevant logic.

Yes, I'm pretty sure core "saw" the attribute because before applied it manually, core added fetchpriority="high" to the first image. When I manually added fetchpriority="high" to the first image, core added it to the second image.

When you encounter the bug, is the image that WordPress adds fetchpriority="high" to before the image that your plugin adds fetchpriority="high" too? If so, that would explain the problem.

In my test, I'm pretty sure I set fetchpriority="high" on the first large image and core still added it to the second image.

Regardless - I feel we should probably avoid adding fetchpriority="high" to any image if the content already contains an image with fetchpriority="high". What do you think? We could avoid this with a content check early, before adding it.

Here is a test post where you can see the behavior: ​https://wp-sample-pages.instawp.xyz/with-manually-set-high-and-low/

[flixos90]

@adamsilverstein We can see if we can find a solution to that problem, but at the moment, due to the sequential processing of images, we can't update an image that was already given fetchpriority="high" when later another image already had the attribute present from a plugin. We would need to somehow parse the entire content in advance, which I'm not sure is feasible unless we apply output buffering - especially since the problem could go beyond images in the post content, these images could appear anywhere.

Based on where we are now, I'm curious whether you could apply the new 6.4 filters in your plugin to ensure the fetchpriority="high" attribute is only on the one image that it was added to. See ​https://make.wordpress.org/core/2023/10/18/image-loading-optimization-enhancements-in-6-4/#:~:text=You%20could%20use%20the%20wp_get_loading_optimization_attributes%20filter%20to%20ensure%20a%20specific%20image%20within%20post%20content%20receives%20the%20fetchpriority%3D%22high%22%20attribute%20while%20other%20ones%20do%20not%3A for a related reference example.

[adamsilverstein]

Good suggestion about adding filters in the plugin to prevent the duplication. I’ll give that a try and also keep thinking about how we could do better automatically.

[flixos90]

@adamsilverstein Checking in here. Two things:

• In ​https://wp-sample-pages.instawp.xyz/with-manually-set-high-and-low/, which of the images is the one you manually added fetchpriority="high" for?
• Regarding your last comment, have you been able to see about using the new 6.4 filters?

[adamsilverstein]

In ​https://wp-sample-pages.instawp.xyz/with-manually-set-high-and-low/, which of the images is the one you manually added fetchpriority="high" for?

The first, core adds it to the second.

Regarding your last comment, have you been able to see about using the new 6.4 filters?

Not yet, thanks for the reminder.

[joemcgill]

@adamsilverstein have you been able to verify whether the filters provide your plugin with the necessary approach to addressing this issue? Not seeing movement on this for a while, I'd suggest we move out of the 6.5 milestone unless you're planning to take a look at this soon.

[swissspidy]

I'd suggest we move out of the 6.5 milestone

Agreed. Feel free to move back once there is a patch & owner.

[adamsilverstein]

I did some further testing on this issue.

I created a test post where a small grid of thumbnails proceeds a large image. On desktop, this image is the LCP element and by default core does a very poor job of tagging this layout. the first thumbnail (which is actually sized "medium") get fetchpriortity="high" and the large image gets loading="lazy" - pretty much the opposite of what we want! see fetchpriority problem.jpg​. ​Test post.

Next I created the same layout, but editing the page code I manually edited the page to add fetchpriority manually to the large image. Viewing the ​new test post I see that the fetchpriority is in the correct place, the first image no longer gets the fetchpriority="high" and the LCP image no longer gets the loading="lazy". See manual fetch priority on large image.jpg​

To summarize, the issue I was seeing here was an artifact of adding fetchpriority via the block_render hook. Once I tested adding the attribute to the markup manually, core behaved as expected. Any fix is probably appropriate in the upstream plugin.