Fix Tinter.setTheme to not fire using Firefox #6831
Conversation
This if checks if we got a Firefox using a variable that is undefined everywhere except in Firefox. In Firefox because of how it renders the DOM ensure that css is always loaded before it loads/runs the js code. Therefor onload 1. never triggers and 2. we can just call setTheme.
|
The only thing I'd probably say is to use something more substantial than |
|
@Half-Shot After searching a few hours and looking for libs most of them did UserAgent detection whioch can break badly. This typeof approach is the one considered most safe according to the Stackoverflow awnser linked in the next comment. Using feature detection instead. Firefox is the only one which uses this Type so it is unique on Firefox. (Edit: Rephrased to make it more clear) |
|
Link I got this check from: https://stackoverflow.com/a/9851769/4929236 |
src/vector/index.js
Outdated
| @@ -313,9 +313,13 @@ async function loadApp() { | |||
| // FIXME: we should probably block loading the app or even | |||
| // showing a spinner until the theme is loaded, to avoid | |||
| // flashes of unstyled content. | |||
| a.onload = () => { | |||
| if (typeof InstallTrigger !== 'undefined') { | |||
There was a problem hiding this comment.
Hmm. please comment this kind of thing (imagine how confused you'd be if you were reading this file and found it looking at something called InstallTrigger...)
Also, is there not a way to do this based on whether the CSS is loaded or not? I'm thinking something like https://gist.github.com/cvan/8a188df72a95a35888b70e5fda80450d although I tested that and Chrome still thinks the stylesheet has rues before onload is fired. How does Chrome fail if you run the Tinter without the onload? Can we not detect that failure and run the Tinter conditionally on that?
There was a problem hiding this comment.
I sure can add a comment. Sorry for having missed to do that.
the eventListener you linked is afaik the same as doing onload. I tried it in firefox and it still did not work.
I am not sure if we can catch a error on chrome as I think the Tinter doesnt return a error at all. I only saw some console logs but even these dont always trigger on a error
|
@dbkr Added a comment and moved the old one to its correct Position. Please mention if there is still something unclear. |
There was a problem hiding this comment.
Thanks for addressing this, and major apologies for letting it sit this long.
This fixes one problem, but doesn't fix #6424 anymore - I've opened matrix-org/matrix-react-sdk#2214 to fix the remaining issue. I suspect the problem is just a symptom of time, given a lot has happened between May and now.
This if checks if we got a Firefox using a variable that is undefined everywhere except in Firefox. In Firefox because of how it renders the DOM ensure that css is always loaded before it loads/runs the js code. Therefor onload 1. never triggers and 2. we can just call setTheme.