cloudflare / templates

A collection of starter templates and examples for Cloudflare Workers and Pages
https://cloudflareworkers.com
MIT License
1k stars 638 forks source link

Safari issues - Googlefont display issues #42

Closed wpsumo closed 2 years ago

wpsumo commented 5 years ago

Causes display issue for Safari, could only see it appear in Safari.

Google font is not displayed and fallback system font is used in Safari when using this script or the streamer with this feature enabled in the workers.

adriantofan commented 5 years ago

It looks like that the cache key computation algorithm has some issue. Just returning the user agent solves the problem. It might be a better solution, but I imagine that it requires keeping up to date with the algorithm google uses to compute the css from the user agent.

wpsumo commented 5 years ago

@adriantofan Okay but why is this only happening on Safari and how do you think we could solve it?

adriantofan commented 5 years ago

When the service worker proxyes a request to google fonts, it sends the original user agent. Then it stores the answer in a cache keyed by a function of the user agent. It looks like this key function is not specific enough, given that the answer from google is tailor made to the user agent. The solution that I tested is to use the complete user agent as a key. It might be possible to determine a “optimal” key function, that works, by testing rigorously with all browsers and os combinations. The author of this code most probably already did that or part of it. Either the testing was not complete or the google algorithm changed. Fixing it would mean to do that again, rigorously and deduce a new key function. This is probably fragile given potential google algorithm evolution. The quick win is obviously to use the complete UA as key. It is also pretty robust, but I don’t know the impact on performance.

350d commented 5 years ago

@alriksson adriantofan Confirmed, returning UA fixes the problem! Thanks!

wpsumo commented 5 years ago

Great, let me know when the streaming is updated!

350d commented 5 years ago

I've found the issue with UA parsing with regexp. Check for updated version

wpsumo commented 5 years ago

Great! I can no see there is any version change in the streamer? Did you update that one as well?

350d commented 5 years ago

@alriksson streamer? i'm not sure i get the point what you mean exactly. I'm just using this js source in my worker on CF and thats it.

wpsumo commented 5 years ago

@350d I use all 3 together which is a streamer created by the same authors. Which includes following:

const ENABLE_PROXY_THIRD_PARTY = true;  // Proxy 3rd-party Javascript and CSS
const ENABLE_GOOGLE_FONTS = true;       // Fast Google Fonts
const ENABLE_EDGE_CACHE = true;         // Edge-cache Wordpress HTML in conjunction with the "Cloudflare Page Cache" plugin
const ENABLE_REWRITE_DOMAINS = true;    // Rewrite well-known CMS static domains (JetPack, shopify, bigcommerce, etc)
wpsumo commented 5 years ago

Can not find the github for the streamer though. Do you want me to send the whole streamer?

350d commented 5 years ago

Here it is https://github.com/pmeenan/cf-workers/tree/master/streaming-optimizations

wpsumo commented 4 years ago

@350d Any chance we could add support for display-font: swap; as well? Once the worker grabs my google font it strips the &display=swap from the url. Adding it to the urls with another php functions in wordpress. Since it's coming from a theme.

Could we make support for swap?