MrOtherGuy / fx-autoconfig

Load custom javascript in browser context
Mozilla Public License 2.0
163 stars 10 forks source link

Flash of unstyled content before author sheet loads #40

Open aminomancer opened 10 months ago

aminomancer commented 10 months ago

With the preloadSheet/addSheet method, there seems to be a flash of unstyled content, I'm guessing because some part of the process is not blocking. It's easy to see with some drastic style like

#main-window {
  filter: invert();
}

I noticed it with a style that allows tabs to flex to the full window width:

.tabbrowser-tab[fadein]:not([style*="max-width"]) {
  max-width: var(--tab-max-width, 225px);
}

I've seen this before while working on this, so I guessed that it's the method. It can't be fx-autoconfig itself, since that's what I've been using to load my stylesheet loader (that uses loadAndRegisterSheet currently), which doesn't have a FOUC. Also, if I use windowUtils.loadSheetUsingURIString, it also seems to block loading, eliminating the FOUC. Which makes sense based on its code comment.

I guess there may be some tradeoffs with using one of these methods. But it can make sense for stylesheets that impact the browser appearance immediately on startup to prefer synchronous loading over any potential performance benefits. CSS loads synchronously by default in most applications, so that might be more expected. Though it could also be cool to have a @defer tag or something.

aminomancer commented 10 months ago

I think depending on your hardware, in order to get this to reproduce consistently, you might need to disable the pre-XUL skeleton UI.

user_pref("browser.startup.blankWindow", false);
user_pref("browser.startup.preXulSkeletonUI", false);

The UI details are saved in the windows registry too, I've had to clear them on many occasions at HKCU\SOFTWARE\Mozilla\Firefox\PreXULSkeletonUISettings

MrOtherGuy commented 10 months ago

Hmmm. This sort of sounds like maybe the injection needs to happen before DOMContentLoaded event. All window scripts and styles are injected at that point and currently there isn't a way to configure this.

Either that, or indeed the sheet preloading step we do is the thing causing this.

How exactly were you injecting your style?

I just now tried with a modified tryLoadScriptIntoWindow method where author style are added to first window using windowUtils.loadSheet() and we just start the preloading after it so new windows still get the preloaded sheet.

However, this seems to make no difference - I can still see a short FOUC - usually at least, but not always. It's super inconsistent, sometimes I can't see the FOUC even with the preloading. This is with the two prefs you mentioned.

aminomancer commented 10 months ago

Ah crap, I forgot my author sheet loader had that @backgroundmodule tab. So yeah once I make it a window script, I get FOUC regardless of which approach I use.

So, I just tried moving the script loading logic into the domwindowopened callback, and that didn't resolve it either. Even when using the stylesheet service over preloadSheet.

I do know there's a slight delay with preloadSheet/addSheet vs. loadSheet, from what I was working on with FeatureCallout. But that was a much shorter delay, like barely a frame, that caused problems with transitions. I just did some testing on that and found I was also able to fix it by using <link> elements instead of loadSheet. But all of that pales in comparison to the delay from waiting for the window.

That's rough. Maybe I need to find some way to make pre-xul skeleton UI work for me. It looks horrendous with my theme. For some reason it wants to use the light mode version too. Anyway, I think the reason I'm able to see the delay so consistently is because of the sheer amount of scripts and stylesheets I have, coupled with my session store holding like 20MB of 500+ tabs or something. So there's a whole lot of stuff holding up the event loop.

MrOtherGuy commented 10 months ago

How about if you run the style injecting directly in the observer callback? Basically run this:

if(this.styles.length > 0){
  const disabledScripts = getDisabledScripts();
  for(let style of this.styles){
    if(!disabledScripts.includes(style.filename)){
      ScriptData.tryLoadScriptIntoWindow(style,aSubject)
    }
  }
}

Immediately before line 568 here

(and remove lines 443-450). I mean aren't observer callbacks pretty close to synchronous, whereas event callbacks might fire at next event loop? I have really hard time reproducing the FOUC consistently so I can't really say if this is any better.

I think it might be fine to make the loader inject author styles at that time if that seems to work better.

aminomancer commented 10 months ago

That's the same thing I did last night. No dice. I'm sure it does make some improvement (how could it not, moving from DOMContentLoaded to domwindowopened), but just not a noticeable amount for me.

However, it just occurred to me I was testing all this with a modified version of my script, not with actual stylesheets. And that is pretty close to the end of the list since its name starts with userChrome. I just didn't want to recreate the stylesheets again after just tossing them out so recently haha. But I guess it could make a significant difference having a ton of other scripts ahead in line