martok / palefill

Inject Polyfills for various web technologies into pages requiring them
https://martok.github.io/palefill/
Mozilla Public License 2.0
79 stars 9 forks source link

Fixing SeaMonkey compatibility and improving few sites #11

Closed UCyborg closed 2 years ago

UCyborg commented 2 years ago

I hope someone finds these beneficial. Take it or leave it.

Edit: Actually, hold on. Reddit fix is not correct. One portion of the code further from regular expression has to be written differently. I'll do it later.

UCyborg commented 2 years ago

Re-raising of non-matched exceptions: Correct me if I'm wrong, does this mean I just add "else throw e;" like shown here?

matchAll copyright: I think it's from SeaHOH (https://github.com/JustOff/github-wc-polyfill/commit/1b0e52569a430dabd9d7d680cbb6f4209e77a377), at least the code doesn't look similar compared to another popular implementation (https://github.com/es-shims/String.prototype.matchAll). I'll add the notice.

YouTube: You're right, window.customElements is actually defined with default user agent Pale Moon spoofs for it, haven't noticed, thought it does non customElements way in that case. Another funny thing, SeaMonkey says by default in its user agent that it's both Firefox 68 and SeaMonkey and then those preview thumbnails work (unlike with UA that just says Firefox 60), video description section has different appearance and customElements is also defined in this case by YouTube. Seems the only case when it isn't defined is with UA of Firefox 65 or newer.

I like the latter combination simply because then it seems to behave like it does on popular browsers. But yes, since the behavior is quirky, no problem, I'll throw that commit out and just use it on my locally installed copy.

BTW, is it OK if I just rebase and amend what's needed and force push to my repo? I'd prefer to have the history that isn't cluttered more than it has to be. I'm not concerned about someone forking my copy and getting confused later (don't think it'll come to that in this case), but will anything get messed up regarding this pull request?

rofl0r commented 2 years ago

but will anything get messed up regarding this pull request?

no, a pull req is just getting updated after a force push with latest branch contents

martok commented 2 years ago

Re-raising of non-matched exceptions: Correct me if I'm wrong, does this mean I just add "else throw e;" like shown here?

As far as I can tell, yes. I remember problems with wrong stack traces, but nobody mentions that anymore, so maybe that's a thing of the past.

YouTube: You're right, window.customElements is actually defined with default user agent Pale Moon spoofs for it, [...] I like the latter combination simply because then it seems to behave like it does on popular browsers. But yes, since the behavior is quirky, no problem, I'll throw that commit out and just use it on my locally installed copy.

Hm, interesting. I didn't take the spoofed UA into account. I guess a way to resolve that for the future could be to add a rule option for the user agent and use that to have rules that only activate on one browser.

Something odd: I have just tested your suggested change again and it isn't that much slower anymore. When I last tested this about 3 months ago (just after the new design was rolled out), cold page load times for video pages went from 12 seconds to >20 seconds, that doesn't seem to be the case anymore. Edit: also, I get the fly-out-cards of the new design in PM without additional changes, looks like this got switched over fully at some point. I'll revisit that (tracking in #12), but it needs a bit more testing to see what else might break without their polyfills. Leave it out of this one for now, please.

BTW, is it OK if I just rebase and amend what's needed and force push to my repo? I'd prefer to have the history that isn't cluttered more than it has to be. I'm not concerned about someone forking my copy and getting confused later (don't think it'll come to that in this case), but will anything get messed up regarding this pull request?

As @rofl0r said, that just works. Github keeps a copy of the old review comments, but the PR itself always works with the current branch state.

martok commented 2 years ago

Perfect, thank you!

UCyborg commented 2 years ago

Looks like Stack Exchange updated their script so it doesn't have the incompatible code anymore.

martok commented 2 years ago

Always interesting to see when and how companies change their babel presets to a higher language level and then change back when it turns out not all their userbase is on V8.

jchwdv commented 1 year ago

Thank you both. You make my SeaMonkey usable with github again. Thank you :+1: