search

wp-media / wp-rocket

Performance optimization plugin for WordPress
https://wp-rocket.me
GNU General Public License v2.0
698 stars 216 forks source link

Google Fonts combining should be disabled for AMP pages #4563

Closed swissspidy closed 2 years ago

swissspidy commented 2 years ago

Before submitting an issue please check that you’ve completed the following steps:

Describe the bug

Google Fonts combining adds onload attributes to the link tags, which causes AMP invalidation, for example when using the Web Stories WordPress plugin.

This is done here:

https://github.com/wp-media/wp-rocket/blob/68459b83c9ab16e27d78ad6a3d33fdedfe942012/inc/Engine/Optimization/GoogleFonts/AbstractGFOptimization.php#L119

To Reproduce Steps to reproduce the behavior:

  1. Enable Google Fonts combining
  2. Visit an AMP page like a web story
  3. Check the source code / run it through AMP validator
  4. See error / onload attribute

Expected behavior

The AMP compat class should be extended to disable Google Fonts combining:

https://github.com/wp-media/wp-rocket/blob/290c89ed141b814641bae71b5238b46498c1f4e8/inc/ThirdParty/Plugins/Optimization/AMP.php

Screenshots If applicable, add screenshots to help explain your problem.

Additional context Add any other context about the problem here.

Backlog Grooming (for WP Media dev team use only)

NataliaDrause commented 2 years ago

Related tickets: https://secure.helpscout.net/conversation/1717271304/312671 https://secure.helpscout.net/conversation/1718076149/312826?folderId=3864740

Issue is also reported here: https://github.com/google/web-stories-wp/issues/9896

viobru commented 2 years ago

Related ticket: https://secure.helpscout.net/conversation/1724424648/313860/

remyperona commented 2 years ago

Scope a solution ✅

We can do as we did for the other options, and use the filter to disable the option when on AMP pages in the disable_options_for_amp() method.

add_filter( 'pre_get_rocket_option_minify_google_fonts', '__return_false' );

Estimate the effort ✅

Effort [XS]

alfonso100 commented 2 years ago

@Tabrisrp I think the method name is disable_options_on_amp()? https://github.com/wp-media/wp-rocket/blob/trunk/inc/ThirdParty/Plugins/Optimization/AMP.php#L111

Mai-Saad commented 2 years ago

Well for Web stories 1.15.1 (while AMP is activated 2.2.0) => GF optimization is not applied while for Web Stories 1.16.0, GF optimization is applied now on develop @alfonso100 @Tabrisrp can you please check this (not sure why optimizations are there for 1.16.0)