Skip to content
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

Allow fetchpriority=high to be added to IMG when it is the LCP element on desktop and mobile with other viewport groups empty #1710

Closed
westonruter opened this issue Nov 29, 2024 · 1 comment · Fixed by #1723
Labels
[Plugin] Image Prioritizer Issues for the Image Prioritizer plugin (dependent on Optimization Detective) [Plugin] Optimization Detective Issues for the Optimization Detective plugin [Type] Enhancement A suggestion for improvement of an existing feature

Comments

@westonruter
Copy link
Member

westonruter commented Nov 29, 2024

Image Prioritizer currently removes or doesn't add fetchpriority=high to an IMG unless every viewport group is populated and each has an LCP element with the same XPath. I think this may be too conservative, especially since many sites may get very few phablet and tablet visitors meaning those URL Metric groups never get populated. I think we should perhaps align with what we're now doing for lazy-loading (#1604), where it does so if the narrowest viewport group (i.e. mobile) and the widest viewport group (i.e. desktop) both are populated. In the same way, if the mobile viewport group and the desktop viewport group both have the same LCP element (i.e. the same XPath) then we can in this case add fetchpriority=high to the IMG itself. Naturally, if the phablet and/or tablet viewport groups are also populated and they record a different LCP element, then fetchpriority=high should still be omitted.

In practice this won't really impact most visitors since the fetchpriority=high preload links area being added for each populated viewport group, but if there were no URL Metrics gathered for phablet/tablet and there is a shared LCP element on mobile & desktop, then there would not be any preload link for the phablet/tablet device. In this case, the presence of fetchpriority=high on the IMG itself would ensure they still get a performance improvement. This is also especially important for the case of optimizing PICTURE elements which have art-directed SOURCE images (with media attributes), in which case adding preload links is not feasible.

Previously discussed at #1707 (comment).

@westonruter westonruter added [Plugin] Image Prioritizer Issues for the Image Prioritizer plugin (dependent on Optimization Detective) [Type] Enhancement A suggestion for improvement of an existing feature labels Nov 29, 2024
@westonruter westonruter added this to the image-prioritizer n.e.x.t milestone Nov 29, 2024
@github-project-automation github-project-automation bot moved this to Not Started/Backlog 📆 in WP Performance 2024 Nov 29, 2024
@westonruter westonruter added the [Plugin] Optimization Detective Issues for the Optimization Detective plugin label Nov 29, 2024
@westonruter
Copy link
Member Author

westonruter commented Nov 30, 2024

This change probably makes sense in OD_URL_Metric_Group_Collection::get_common_lcp_element() directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Plugin] Image Prioritizer Issues for the Image Prioritizer plugin (dependent on Optimization Detective) [Plugin] Optimization Detective Issues for the Optimization Detective plugin [Type] Enhancement A suggestion for improvement of an existing feature
Projects
Status: Done 😃
1 participant