Open Bug 1437644 Opened 6 years ago Updated 8 months ago

Flash of unstyled content with Imagus addon (FOUC)

Categories

(WebExtensions :: Compatibility, defect, P5)

Product:
WebExtensions
Component:
Compatibility
Type:
defect

Tracking

(Not tracked)

People

(Reporter: bzbarsky, Unassigned)

References

(Blocks 1 open bug)

Details

Per bug 1404468 comment 29, installing https://addons.mozilla.org/en-US/firefox/addon/imagus/ and loading https://www.mysstie.com/systeminfo.shtml should lead to FOUC ("should" because I have not tried it yet).
Blocks: 1404468
OK, we have .clientWidth get on the <html> element.  This flushes out layout, which causes us to have to render the page.  This .clientWidth get is coming before the sheet is done loading.

The JS stack to that call is:

0 viewportDimensions() ["moz-extension://d35d8824-3d12-45b7-bc9d-6f0bdc980a49/includes/content.js":8]
    this = [object Window]
1 init(e = [object Object]) ["moz-extension://d35d8824-3d12-45b7-bc9d-6f0bdc980a49/includes/content.js":139]
    this = [object Window]
2 callback(args = [object Object]) ["resource://gre/modules/Schemas.jsm":2289]
3 apply(target = (...args) => {
            this.checkCallback(args, context);
            original(...args);
          }, thisArgument = null, argumentsList = [object Object]) ["self-hosted":4489]
    this = [object Object]
4 applySafeWithoutClone(callback = (...args) => {
            this.checkCallback(args, context);
            original(...args);
          }, args = [object Object]) ["resource://gre/modules/ExtensionCommon.jsm":300]
    this = [object Object]
5 applySafeWithoutClone((...args) => {
            this.checkCallback(args, context);
            original(...args);
          }, [object Object]) ["self-hosted":998]
6 wrapPromise/<(args = [object Object]) ["resource://gre/modules/ExtensionCommon.jsm":473]

so it's coming from inside the Imagus addon, unsurprisingly.  So there are two questions:

1) When exactly is this being called?
2) What is it doing?

It looks like it's called from an init() function that we call from inside browser code.  Kris, do you know offhand when we would be coming through that code?

That init() function calls viewportDimensions(), which does:

        var d = targetDoc || doc;
        d = d.compatMode === "BackCompat" && d.body || d.documentElement;
        var w = d.clientWidth;
        var h = d.clientHeight;
        if (targetDoc) return {
            width: w,
            height: h
        };
        if (w === winW && h === winH) return;
        winW = w;
        winH = h;
        topWinW = w;
        topWinH = h

I guess the extension is trying to exclude the viewport scrollbars, hence not using window.innerWidth/Height.  But since it's running this before stylesheets are loading, it's probably getting the wrong answer sometimes...
Flags: needinfo?(kmaglione+bmo)
This is being triggered from the extension's own code. It adds a message listener that calls PVI.init:

        Port.listen(PVI.init);
        Port.send({
            "cmd": "hello"
        })

And then its background script sends it a message.
Flags: needinfo?(kmaglione+bmo)
I wonder if we could detect FOUCs caused by add-on code and show an info-bar asking if we should delay loading the add-on's content scripts until after first paint...
Hmm.  So wait, is it setting up a listener and then immediately dispatching the relevant message?

It looks like this is basically running at toplevel in includes/content.js, right?  When do we normally run that?  Is it basically "at global setup time", and so all that's happening is they're running one event loop spin after that?
> I wonder if we could detect FOUCs caused by add-on code

Oh, and ... we could, sort of.  We could detect FOUCs that happen when addon code is the closest JS on the stack.
(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #4)
> Hmm.  So wait, is it setting up a listener and then immediately dispatching
> the relevant message?
> 
> It looks like this is basically running at toplevel in includes/content.js,
> right?  When do we normally run that?  Is it basically "at global setup
> time", and so all that's happening is they're running one event loop spin
> after that?

The add-on injects a content script into all pages at document_end (basically DOMContentLoaded), which is when the code that adds the listener runs. The content script then sends a message to its background script, which is (usually) in a different process, and its response triggers the init() function.

Exactly how much later that happens depends on a lot of factors, but it looks like the background script always responds to the message immediately, so it should be relatively quick.
For comparison sake, this page: https://www.markheadrick.com/systeminfo.shtml doesn't do it and it's almost the same content but is loading a javascript at the bottom.  Hmmmm.. some of that HTML code is like 20 years old LOL.  I maintain both sites so can make any changes for testing purposes.. or create some extra test pages.
> The add-on injects a content script into all pages at document_end (basically DOMContentLoaded)
...
> but is loading a javascript at the bottom.

Right.  DOMContentLoaded fires when parsing is done.  It does not wait for stylesheet loads.  It does wait for script executions, because scripts can do document.write(), which can affect parsing.

Also, script execution waits for stylesheet loads from earlier in the document, because scripts can request layout information, which depends on styles.

So if you add a script at the bottom, it will wait for the sheet and block DOMContentLoaded, and that means the addon code will not run until after the stylesheet has loaded.

The right general solution for Imagus seems to be for us to give them a notification at the point when parsing is done _and_ sheets are loaded.  For most pages, which have a <script> after any styles they have, this will be DOMContentLoaded anyway....

Another option is for us to change parsing to block on stylesheets, which will obviously eliminate FOUC altogether.  I've been sort of pushing back on that, because it reduces available parallelism.
Component: DOM → Extension Compatibility
Product: Core → Firefox
Summary: Flash of unstyled content with Imagus addon → Flash of unstyled content with Imagus addon (FOUC)
(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #8)
> The right general solution for Imagus seems to be for us to give them a
> notification at the point when parsing is done _and_ sheets are loaded.  For
> most pages, which have a <script> after any styles they have, this will be
> DOMContentLoaded anyway....
> 
> Another option is for us to change parsing to block on stylesheets, which
> will obviously eliminate FOUC altogether.  I've been sort of pushing back on
> that, because it reduces available parallelism.

We can probably change document_idle scripts to run after stylesheets have loaded. document_idle scripts are defined to run sometime between DOMContentLoaded and load, when the event loop is idle. Waiting for stylesheets to lpad shouldn't break that contract. The add-on would still need to be changed to use that event, though (which it really probably should, anyway).
> We can probably change document_idle scripts to run after stylesheets have loaded.

Good idea.  Filed bug 1437921.  Leaving this one for tracking the change to the addon.
Component: Extension Compatibility → WebExtensions: Compatibility
Product: Firefox → Toolkit
Looks like Imagus v0.9.8.61 has a workaround and it doesn't cause this issue for me on the sites that it did before.
per comment 10 left open for tracking change to add-on, but from bug fixing perspective - if this comes up again close as "works for me".
Priority: -- → P5
I get FOUC on https://www.mysstie.com/systeminfo.shtml in FF58 and no FOUC in FF60 beta regardless of whether Imagus is active or not.
Product: Toolkit → WebExtensions
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.