-
Notifications
You must be signed in to change notification settings - Fork 109
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix handling of image prioritization when only some viewport groups are populated #1404
Fix handling of image prioritization when only some viewport groups are populated #1404
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
…rt group is populated
Ugh. Spell Check is complaining about the lorem ipsum 😅 |
I deployed this change to my site and it fixed the problem. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Glad you don't have tablet visitors, allowing you to find this bug! 😄
I discovered an issue on my blog in which the most recent blog post lacks a featured image but the second-most recent post does have a featured image. The featured image on the second post is getting
fetchpriority=high
added by server-side heuristics but Image Prioritizer was not removing this even though the image was not visible on the mobile viewport, although it is correctly the LCP element on desktop:My URL metrics were fully populated on the mobile and desktop viewport groups, but they are empty for phablet and tablet viewport groups. I don't get a lot of traffic on my site, and apparently nobody with a phablet or tablet visits my site. So I have no metrics for them:
URL Metrics Dump
Note in particular:
The logic here is incorrect:
performance/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php
Lines 55 to 67 in 3364bd9
Instead of removing
fetchprority=high
when all viewport groups are populated (which may never happen, as in the case of my blog), it should be removed when any of the viewport groups are populated. If none are populated, then the attribute should be left alone to let the server-side heuristics apply as a fallback behavior.