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

Stop WP rocket preloading all fonts it finds in the css when 'Remove unused css' is turned on #6202

Closed
be6945 opened this issue Oct 4, 2023 · 16 comments · Fixed by #6445
Closed
Assignees
Labels
community Issues created by someone outside of our team effort: [XS] < 1 day of estimated development time needs: documentation Issues which need to create or update a documentation priority: medium Issues which are important, but no one will go out of business. type: enhancement Improvements that slightly enhance existing functionality and are fast to implement
Milestone

Comments

@be6945
Copy link

be6945 commented Oct 4, 2023

Is your feature request related to a problem? Please describe.
When using the 'Remove unused css' feature, WP-rocket pre-loads all fonts it finds in the CSS files. This causes many unwanted fonts to be pre-loaded (especially those from plugin files) and affects LCP etc.

Describe the solution you'd like
A toggle button on whether to preload fonts it finds or not when using this feature.

Describe alternatives you've considered
Using a different optimization plugin.

@piotrbak piotrbak added the community Issues created by someone outside of our team label Oct 9, 2023
@piotrbak
Copy link
Contributor

piotrbak commented Oct 9, 2023

Hello @be6945 thanks for creating the issue. Could you send me the website which has a worse LCP when preloading the fonts? We'd like to see what's the problem there, you could reach me at piotr[at]wp-media.me.

@thinkjarvisdesignandmarketing
Copy link

thinkjarvisdesignandmarketing commented Oct 31, 2023

@piotrbak
Yes this 100% causes problems. Some fonts are poorly optimised. If all fonts are preloaded and the payload for that can be up to 1mb depending on how well the theme is coded. This will cause fonts to load before the LCP item on the page.

Example here reported on the forum where 319kb of fonts are loaded that arent used above the fold:
I responded telling them to either re-optimise their fonts to reduce the size or remove the from pre-load.

All fonts should not be preloaded by default. There is no logic in that - especially when there is likely only 1 or 2 fonts to load above the fold.

@alixtak
Copy link

alixtak commented Dec 1, 2023

Indeed, every single website I work on loads multiple icon librairies (fontawesome, eicons, ...). Which hugely impacts LCP since we are preloading them. Having a text box to exclude fonts from preloading is becoming a priority on my end.

I would like to know if there's anything planned regarding this, thank you very much.

@tyzberd
Copy link

tyzberd commented Dec 12, 2023

I support it, it greatly affects the LCP metric, especially when there is a font with icons.

@Eroan
Copy link

Eroan commented Jan 4, 2024

Hi, I confirm that this behavior can be really problematic and impact LCP in a bad way.

You should at least provide a hook to blacklist some fonts based on name.

@tomphilpotts
Copy link

I have also had this issue. I am already preloading the fonts that we need. Disabling this feature increased LCP by 10 points.

I did try to use this add_filter('rocket_disable_preload_fonts', '__return_true');, but did not work

@ivanvasqueze
Copy link

ivanvasqueze commented Jan 28, 2024

@piotrbak Sí, esto 100% causa problemas. Algunas fuentes están mal optimizadas. Si todas las fuentes están precargadas y la carga útil puede ser de hasta 1 MB dependiendo de qué tan bien esté codificado el tema. Esto hará que las fuentes se carguen antes que el elemento LCP en la página.

Ejemplo aquí informado en el foro donde se cargan 319 kb de fuentes que no se usan en la mitad superior de la página: respondí diciéndoles que volvieran a optimizar sus fuentes para reducir el tamaño o eliminarlas de la carga previa.

No todas las fuentes deben estar precargadas de forma predeterminada. No hay lógica en eso, especialmente cuando es probable que solo se carguen 1 o 2 fuentes en la mitad superior de la página.

Hola, quisiera pedirte amablemente que utilices un acortador de URL al referirte a mi sitio, ya que estoy trabajando en su optimización para los motores de búsqueda y preferiría que cuando alguien busque el nombre de mi marca, encuentren el contenido de mi sitio web, que ofrece servicios odontológicos, en lugar de esta discusión. Aunque enfrenté problemas de velocidad en el pasado, ahora he pasado la Evaluación de las métricas web esenciales en PageSpeed tanto en dispositivos móviles como en ordenadores. Gracias por tu comprensión.

@camilamadronero
Copy link

Maybe @wordpressfan can help us with this quick request here ?
The user ivanvasqueze needs us to hide the mention to his site in this comment because it's interfering with the search results when people are looking for his site
I guess it's safer and quicker to have the URL removed from the comment
Thanks!

@piotrbak
Copy link
Contributor

Done @camilamadronero @ivanvasqueze

@astbarc
Copy link

astbarc commented Feb 2, 2024

I'm also facing this issue in some websites. With Elementor for example all the font awesome files are preload even if you're not using the font in the page.

Would be great to have a way to exclude fonts for be preloaded.

@piotrbak piotrbak added type: enhancement Improvements that slightly enhance existing functionality and are fast to implement priority: medium Issues which are important, but no one will go out of business. and removed waiting for feedback labels Feb 12, 2024
@Tabrisrp
Copy link
Contributor

We have a filter to completely remove this rocket_enable_rucss_fonts_preload. returning false stops the process. This is a temporary solution for users who need it.

On the long term, this would need to be solved on the SaaS side. What happens on the plugin side is that we get the used CSS sent by the SaaS, parse it to extract the fonts URLs, and preload them. We don't know which ones are currently used on the page.

Ideally, the SaaS would return the used CSS with only the fonts used on the page, and it would solve the issue.

@piotrbak @MathieuLamiot

@piotrbak
Copy link
Contributor

@Tabrisrp sounds like filter with exclusions of specific fonts would be a quick workaround for this problem. It should be an easy thing to implement, right?

@Tabrisrp
Copy link
Contributor

Yes it can be a quick solution that can be used by the users or with an helper plugin

@piotrbak
Copy link
Contributor

Let's go with this approach then for now:

  1. Introduce new filter (rocket_exclude_rucss_fonts_preload, or another, if there's a better idea)
  2. Filter will allow to exclude specific fonts from being preloaded when RUCSS is enabled
  3. It should allow for partial match exclusions, domain.com/fonts wille exclude domain.com/fonts/font.woff
  4. rocket_enable_rucss_fonts_preload should work without regressions

@Tabrisrp
Copy link
Contributor

Scope a solution ✅

Engine\Optimization\RUCSS\Controller\UsedCSS

In add_used_fonts_preload():

  • Add new filter rocket_exclude_rucss_fonts_preload, with a default value of an empty array
  • Inside the loop on all fonts, check if the font URL matches with one of the values inside the filter array
  • If it matches, bail-out before adding the font url to the $urls array

Estimate the effort ✅

Effort [XS]

@Tabrisrp Tabrisrp added effort: [XS] < 1 day of estimated development time and removed needs: grooming labels Feb 12, 2024
@Miraeld Miraeld self-assigned this Feb 13, 2024
@Miraeld Miraeld reopened this Feb 13, 2024
@be6945
Copy link
Author

be6945 commented Feb 13, 2024

Great to see this being accepted as an issue, thank you. Please let us know how to implement too. Thanks again everyone, great work.

@Miraeld Miraeld removed their assignment Feb 21, 2024
@Miraeld Miraeld self-assigned this Feb 22, 2024
@piotrbak piotrbak added this to the 3.15.10 milestone Mar 4, 2024
@piotrbak piotrbak added the needs: documentation Issues which need to create or update a documentation label Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Issues created by someone outside of our team effort: [XS] < 1 day of estimated development time needs: documentation Issues which need to create or update a documentation priority: medium Issues which are important, but no one will go out of business. type: enhancement Improvements that slightly enhance existing functionality and are fast to implement
Projects
None yet
Development

Successfully merging a pull request may close this issue.