litespeedtech / lscache_wp

LiteSpeed Cache for WordPress
http://wordpress.org/plugins/litespeed-cache/
GNU General Public License v3.0
208 stars 110 forks source link

Load Google Fonts Asynchronously - not loading all weights with css2 #679

Open kokers opened 2 months ago

kokers commented 2 months ago

Looks like google fonts are not fully loaded when the embed code uses the new API css2. Only the default weight (400) is being pulled.

Url is build like this: https://fonts.googleapis.com/css2?family=Manrope:wght@200..800&display=swap

Looks like webfontloader doesnt handle css2 correctly. Might need a library update. Found one very old issue still open https://github.com/typekit/webfontloader/issues/430 Not sure when google made a switch, but they generate embed codes now on google fonts with the css2 links.

Old API with links https://fonts.googleapis.com/css?family=Manrope:300,400,600,700,800 work ok.

timotei-litespeed commented 2 months ago

@kokers Are you able to test some changes to the plugin files?

kokers commented 2 months ago

@timotei-litespeed sure, let me pull this commit and test it. Will let you know if that fix it.

timotei-litespeed commented 2 months ago

@kokers quick answer :)) I've been testing the updated that code and seems it works. image image

timotei-litespeed commented 2 months ago

Just try on a staging site. That code is in development.

timotei-litespeed commented 2 months ago

Maybe take just the JS files content

kokers commented 2 months ago

Sorry it took a bit but needed to make sure. Of course doing this on staging no worries.

It doesnt work. I had originally just a code in wp_head hook that is a full embed code generate by google, swapped to the wp_enqueue_style to make sure, but it doesnt work.

You can see that its not, cause in sources the CSS url includes css instead of css2

This is when Load Google Fonts Async is OFF

image

This is when Load Google Fonts Async is On

image

This is the code for font:

wp_enqueue_style('qc-font-Manrope', 'https://fonts.googleapis.com/css2?family=Manrope:wght@200..800&display=swap', [], '1.1');
timotei-litespeed commented 2 months ago

Does the page have multiple google font links?

timotei-litespeed commented 2 months ago

I see the same thing, but the fonts are loaded image image image

kokers commented 2 months ago

They are loaded with only single weight. look at the CSS that has been pulled. it's only 400. All other weights are missing.

image

This is correct source that we should see:

image

It's not that font is not loaded. It's that only default (400) weight is, cause css API cant interpret the :wght@200..800 part. if its loaded with css2 it loads all defined weights in url.

Now it's time to figure out if it's a webfontloader issue, or there is some swap happening in LS cache plugin that changes css2 url to css which causes the issue.

timotei-litespeed commented 2 months ago

That webkit version is oldddd But the font will load and show(LIMITED!).

I tried to add css2(as in other pull requests) but no luck. I am trying to find support for both V2 and V1.

timotei-litespeed commented 2 months ago

New commit: https://github.com/timotei-litespeed/lscache_wp/commit/150c21484a774f6d819863571d84861b98f220a5

How I tested: functions.php

function add_google_fonts() {
    wp_enqueue_style( 'add_google_fonts', 'https://fonts.googleapis.com/css?family=Open+Sans:300,400,800', false );
    wp_enqueue_style( 'add_google_fonts2', 'https://fonts.googleapis.com/css2?family=Manrope:wght@200..800&family=Open+Sans:ital,wght@0,300..800;1,300..800&family=Playwrite+ES+Deco:wght@100..400&family=Playwrite+US+Trad:wght@100..400&family=Poppins:ital,wght@0,100;0,200;0,300;0,400;0,500;0,600;0,700;0,800;0,900;1,100;1,200;1,300;1,400;1,500;1,600;1,700;1,800;1,900&display=swap', [], null );
    wp_enqueue_style( 'add_google_fonts3', 'https://fonts.googleapis.com/css2?family=Bungee+Spice&family=Maname&family=Playwrite+HR+Lijeva:wght@100..400&family=Playwrite+HU:wght@100..400&display=swap', [], null);
    wp_enqueue_style( 'add_google_fonts4', 'https://fonts.googleapis.com/css2?family=Roboto:ital,wght@0,100;0,300;0,400;0,500;0,700;0,900;1,100;1,300;1,400;1,500;1,700;1,900&display=swap', [], null);
    wp_enqueue_style( 'child-style', get_stylesheet_directory_uri() . '/style.css', array());
}
add_action( 'wp_enqueue_scripts', 'add_google_fonts' );

style.css

.font_1{ font-family: "Open Sans"; font-weight: 800; font-size: 44px; font-style: normal; }
.font_2{ font-family: "Manrope"; font-weight: 300; font-size: 44px; font-style: normal; }
.font_3{ font-family: "Poppins"; font-weight: 400; font-size: 44px; font-style: normal; }
.font_4{ font-family: "Playwrite ES Deco"; font-weight: 400; font-size: 44px; font-style: normal; }
.font_5{ font-family: "Playwrite US Trad"; font-weight: 100; font-size: 44px; font-style: normal; }
timotei-litespeed commented 2 months ago

One thing I needed to add is: ....1,900&display=swap', [], null ); change version parameter to value null Testing with LSC off I saw that only first font was loaded, and the rest were ignored. Also updated webfont loader

@kokers sorry for late answer

timotei-litespeed commented 1 month ago

I have revisited my work and the correct test will be from: https://github.com/timotei-litespeed/lscache_wp/commit/e72f969aa1bcc234c9663dace1a2fbfe4a738312

Still the wp_enqueue_style paramers 2 and 3 needs to be present. wp_enqueue_style( 'add_google_fonts2', 'https://fonts.googleapis.com/css2?family=Bungee+Spice&family=Maname&family=Playwrite+HR+Lijeva:wght@100..400&family=Playwrite+HU:wght@100..400&display=swap', [], null); Without them the multiple families will remain 1 only

kokers commented 1 month ago

Will test on Monday and confirm :)

kokers commented 1 month ago

This doesn't work. It pulls the first weight only and not all of them. With the syntax 100...400 it needs to pull 100,200,300,400 so the https://fonts.googleapis.com/css2?family=Manrope:wght@300..800&display=swap needs to become: https://fonts.googleapis.com/css?family=Manrope:300,400,500,600,700,800

Essentially current fix is how it worked in the moment I posted the issue, cause font loader was able to get only the first one and ignored the rest of the syntax.

timotei-litespeed commented 1 month ago

Beside that, is there other bugs?

kokers commented 1 month ago

No, but the issue I submitted is about that.

Only the default weight (400) is being pulled.

So this solution doesnt change anything apart from adding extra code.

timotei-litespeed commented 1 month ago

But if you have italic you have to also get italic 300, italic 400, italic 500...

Yes?

kokers commented 1 month ago

Good catch. yes.

timotei-litespeed commented 1 month ago

This should fix: https://github.com/timotei-litespeed/lscache_wp/commit/2bd5a5338cc93efbfdf6a28f9097588dd304547b

kokers commented 1 month ago

Nice! Looks like it works now, thank you

timotei-litespeed commented 1 month ago

The solution is limited la what CSS v1 can do, but will do the trick for now. We will see what exactly will be the final solution and how we will integrate in future releases.

timotei-litespeed commented 1 month ago

And thank you for testing the solution :)