wp-media
3.19: combined google fonts are there in html while it shouldn't
Describe the bug Scenario1: Given: host google fonts is enabled And: filter to add hosted google fonts inline When: visit page with GF v1, v2 Then: inline google fonts is there And: href for google fonts is there while it shouldn't
Scenario2: Given: host google font is enabled And RUCSS is on When visit page with GF And wait till used CSS is generated Then refresh the page And the hosted gf CSS is there before the used CSS while it shouldn't
To Reproduce Scenario1 Steps to reproduce the behavior:
- Install and activate 3.19
- Enable self hosted google fonts in media tab
- use the filter for inline hosted GF
add_filter( 'rocket_host_fonts_locally_inline_css', function( $flag ){
return true;
} );
- visit page with GF and check source i.e https://new.rocketlabsqa.ovh/combine-v1-v2-fonts
Expected behavior Only inline GF is there and no href for GF CSS
Screenshots
Additional context
- on 3.18.3 , only inline GF is there
- retest shall be on single/multisite
- on 3.19, if filter is not used , we enabled RUCSS and host google fonts => after used CSS created, we will see CSS files of google fonts before the used CSS which wasn't the case on 3.18.3 . If this is not solved here then we will need new GH about that
Reproduce the problem ✅
I was able to reproduce the problem following the exact steps
Identify the root cause ✅
Since we introduced rocket head ordering, we no longer add the combined Google Fonts styles directly into the <head> using the rocket_buffer hook at priority 17.
However, at that same priority (17), we still strip out the original <link> tags for Google Fonts using this logic:
The combined fonts are now inserted into the <head> using the rocket_head_items hook instead, with a much higher priority of 100000:
- GoogleFonts/Subscriber.php#L58-L61
- Hook is defined in Common/Head/Subscriber.php#L37-L40
The issue:
When the original Google Fonts are detected and removed (as seen here), they won’t appear at priority 18 (as expected in Frontend/Subscriber.php#L34).
Instead, they show up at a later point:
-
Frontend/Subscriber.php#L37 — because this uses the same
rocket_head_itemshook we now use to add the combined fonts. But the callback for that hook does not handle removal of the combined Google Fonts.
So what happens: The logic meant to remove combined Google Fonts bails out here:
And then rewrite_fonts_in_head runs:
- Controller.php#L406 …but without removing the combined font tag, as intended.
Scope a solution ✅
~I believe we need to Increase the priority here to be greater than 100000 so that way it will run after the combined fonts have been applied to the buffer and the callback can process it as usual, then refactor the rewrite_fonts to not modify the head but assign the hosted fonts values and detected combined fonts to a property in the class, then get that property in rewrite_fonts_in_head and modify the head with it to remove the combine fonts. This method will need some refactoring.~
I have 2 solutions
1st solution
- Add a conditional here to check if
host_fonts_locallyis enabled and return false. - Add a conditional here to check if
host_fonts_locallyis enabled and addfonts.googleapis.comto the exclusion.
2nd solution
- Remove this event here since the callback will bail out here because the google fonts css have been removed from the buffer at this point.
- Add a conditional in the loop here do a regex match to check href element in the array if it contains googleapis and unset the whole array.
I'm with 1st solution depending on the thread here
Updated test if necessary.
Estimate the effort ✅
[S]
Related Test Plan: https://wpmediaqa.testrail.io/index.php?/runs/view/1049
