Skip to content
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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

pjpscriv
Copy link

@pjpscriv pjpscriv commented Feb 9, 2025

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:

<link as="style" href="https://assets.quizlet.com/_next/static/css/c942139eed6c8f57.css" rel="preload">

In particular, I found the script failed at line 25 of content.js, and debugging in dev tools, it looks like these rel="preload" css files for whatever reason don't let you use sheet.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 function fetchCSSRulesFromUrl).

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:

  • Togglable logging functionality to help with debugging
  • Changing how often the #country-flag-fixer-ext tag is updated:
    • Now updates once per CSS sheet as opposed to once per mutation cycle
    • Before removing the old #country-flag-fixer-ext tag, its existing cssRules are read, added to the new tag, and then the new CSS rules are added
    • I did this because I found if I didn't, a CSS file would get read correctly, and the rules would be ready to add to #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 added
    • This now means the main function the observer calls is updateFontFamilyRules() instead of applyCustomFontStyles()
  • Pulling out styleSheetsChanged into its own function

Testing

This fix works for nearly all the broken sites mentioned in current issues except Bluesky.

Checked against the following URLs. Screenshots below.

Quizlet ✅

image

Strava ✅

image

Letterboxed ✅

image

Old reddit ✅

image

Bluesky ❌

image

Emojipedia ✅

image

@pjpscriv pjpscriv changed the title Add country flag fix font to CSS rules that exist files by URL that wouldn't otherwise be available on the document Add country flag fix font to CSS rules that exist in externally loaded files Feb 9, 2025
@pjpscriv pjpscriv changed the title Add country flag fix font to CSS rules that exist in externally loaded files Fetch CSS files that can't be accessed directly Feb 9, 2025
@pjpscriv
Copy link
Author

pjpscriv commented Feb 9, 2025

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.

@matthijs110
Copy link
Owner

matthijs110 commented Feb 10, 2025

Hi @pjpscriv , first of all: thank you very much for the time and effort you put into investigating and addressing the issues!
My apologies for not responding to the issues reported so far. I just couldn't find the right time and motivation to work on this project. Not only because of other priorities, but also because I got stuck with some issues, where each "solution" creates issues elsewhere.

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!

@KarlLee830
Copy link

Very much looking forward to fixing the Youtube issue! Looking forward to your PR!

@KarlLee830
Copy link

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.

@pjpscriv
Copy link
Author

@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. youtube.com, linkedin.com, etc) where the extension would be disabled (and maybe allow users to add other sites to this list if they find other websites that perform poorly with the extension). But with KarlLee's comment now I'm wondering if instead of fully disabling, the extension could instead fall back to using the old approach for these non-performant sites. Obviously "if the website you're visiting is in this list then revert back to the old method" isn't ideal and it would be better to have some smart way of distinguishing between "can use the new method" and "is slow and should use the old method", but I can't think of a smarter way to do it at the moment - maybe some sort of timeout?

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.

@matthijs110
Copy link
Owner

matthijs110 commented Feb 14, 2025

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Not working for quizlet.com
3 participants