-
Notifications
You must be signed in to change notification settings - Fork 5
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
Fetch CSS files that can't be accessed directly #19
base: master
Are you sure you want to change the base?
Conversation
Hmm probably don't want to merge this immediately - as a first impression browsing it seems to make #13 worse and slows LinkedIn all the way down too. Will need some performance optimizations before merging. |
Hi @pjpscriv , first of all: thank you very much for the time and effort you put into investigating and addressing the issues! In fact: I have disabled this extention for myself, because YouTube became very slow (yes, I am very well aware of this issue 😅). Its frustrating, as you might have experienced yourself by now as well 😉 Its very hard to find a one-size-fit-all solution. Mainly because there are so many ways websites are rendered. I will take a proper look into your PR when its ready! |
Also package.json version bump
Very much looking forward to fixing the Youtube issue! Looking forward to your PR! |
Also I would like to mention that when I use this Grease Monkey script modified from this plugin, YouTube doesn't have problems with loading speed. |
@KarlLee830 thanks! And interesting, if I'm reading the code correctly it looks like that userscript is using the approach from a previous version of the extension - version 1.2 I think? I had a little play around with it, and it looks like it doesn't fix flags for some of the problematic websites already noted (Strava, Quizlet, etc), but it definitely got me thinking. @matthijs110 cheers for the reply 😊 and apologies for the delayed response! Do you have any advice on approaches you've tried / what might be a good way to attack this? To do some thinking out loud: My first thought was to take a naive approach and simply have a blacklist of sites (e.g. Anyway that's my braindump - sorry for the wall of text 😅What do you reckon? Also, a side note: it looks like the Chromium bug ticket for this issue that was previously closed has been re-opened now - so maybe at some point there will be a fix for this from the browser side 🤞 Note to self: for future testing, the comments of this youtube video have a good number of flag emojis. |
My original thinking a few months ago was to support multiple 'versions' of this extension, but this is where time (and later) motivation started to be an issue 😅 Hear me out, because its very close to your suggestion! As you also noticed, older versions didn't have much issues on certain websites, where the current version does, and vice versa. Some sites where slower on one version, where other sites just didn't work at all. This got me thinking: what if I give the user a choice? And even beter: let the user control this per website. This is possible as you can simply build a configuration page or popup. On top of that, even if certain sites keep bugging with this extention, users can simply add the website to a blacklist. Just like you were suggesting. I didn't want to exclude sites by default, but leave this choice to the end user. Even though automatically deciding which version to apply is best, I don't think there is a solid way of "meassuring" the performance. Too many factors are in play here (overal performance of the website, different ways of rendering, general browser performance, internet speed, etc.). A timeout could help, but this means you'd have to completely re-render the website to make sure each element has been refreshed (correct me if I'm wrong). Storing this information for each site could be an option though, where only the first time rendering the website could be 'problematic'. It will take some time to build, investigate and thoroughly test all of this. Feel free to give it a shot! Thanks for sharing your thoughts! |
Fixes #18, #16, #14, #10, #8.
Context
When investigating #18, I found that CSS files that are linked linked with
rel="preload"
, didn't appear to have their font-family overrides correctly added to the#country-flag-fixer-ext
style tag that the extension adds. For example:In particular, I found the script failed at line 25 of
content.js
, and debugging in dev tools, it looks like theserel="preload"
css files for whatever reason don't let you usesheet.cssRules
(like line 25 uses).So I added in a try/catch (here) that first tries to read the rules with
sheet.cssRules
, and if this fails, then it will try to retrieve the CSS file directly from the href (new functionfetchCSSRulesFromUrl
).This is the core of the change which gets the missed styles added into the
#country-flag-fixer-ext
style tag.Other changes done alongside this change:
#country-flag-fixer-ext
tag is updated:#country-flag-fixer-ext
tag, its existingcssRules
are read, added to the new tag, and then the new CSS rules are added#country-flag-fixer-ext
, but then if there was a CORS or some other error trying to read a later CSS file, this would error and exit the whole process meaning the rules from the previously successfully read file wouldn't be addedupdateFontFamilyRules()
instead ofapplyCustomFontStyles()
styleSheetsChanged
into its own functionTesting
This fix works for nearly all the broken sites mentioned in current issues except Bluesky.
Checked against the following URLs. Screenshots below.
Quizlet ✅
Strava ✅
Letterboxed ✅
Old reddit ✅
Bluesky ❌
Emojipedia ✅