1462070 - Extensions that apply user CSS don't do it the same way as userContent.css does (FOUC)

Status: RESOLVED INCOMPLETE

(WebExtensions :: Developer Outreach, defect)

Description

[Éter]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.139 Safari/537.36

Steps to reproduce:

1. Install a extension that applies user CSS, like Stylus or re-Style.
2. Add a dark background CSS rule into it to affect www.google.com
3. Expect a white flash when loading the site.


Actual results:

The user CSS from extensions are not loaded the same way as the user CSS written into userContent.css. This leads to white flashes when using dark styles on white background sites.


Expected results:

In Firefox there is this problem that is common to every extension that applies user CSS, like Stylus, reStyle or Dark Reader, and nobody has solved it yet. The problem is that some sites emit white flashes when you use a dark CSS user style for a site with a white background.
Examples: https://www.google.com/
https://www.reddit.com/
http://www.wordreference.com/es/en/translation.asp?spen=hola

I have looked into the problem and I have verified that writing a dark CSS style into an extension like Stylus causes the flashes, but writing the same exact dark style into Firefox's userContent.css resolves the flashes.
The conclusion that I have come into is that the way that extensions like Stylus, re-Style or Dark Reader apply the styles in Firefox is different than the way that userContent.css applies the styles, being that the way to go.

Finally, I don't know if user CSS extensions could apply the styles in Firefox the same way that userContent.css does. That is way I am writing this, to read experts opinion.

Comment 1

[Éter]

This problem was not present on Firefox version 52.5.0.

Comment 2

[Gingerbread Man]

Sounds like a "flash of unstyled content" issue.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Well...

Flash of unstyled viewport, right?  What is the mechanism extensions actually use to apply styles here?

Comment 4

[Andrew Swan [:aswan]]

If this is the Stylus source code:
https://github.com/openstyles/stylus

then the extension appears to inject a content script into every page that sends a message to the background page asking what styles to apply.  This is always going to be racy (in addition to being needlessly expensive).

Using an API like this instead would be more efficient and would make styles available before the content is first displayed:
https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/contentScripts/register

Comment 5

[Kris Maglione [:kmag]]

(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #3)
> Well...
> 
> Flash of unstyled viewport, right?  What is the mechanism extensions
> actually use to apply styles here?

I believe that is the case, yes. Or possibly flash-of-unstyled-about:blank.

We asynchronously load extension stylesheets when we detect a page load that needs them (usually starting with the network request), so they're usually available pretty early in the load cycle. And since bug 1438974, we block parsing when the document element is inserted until those stylesheets are injected to make sure we don't get a FOUC for page content.

The flash of viewport styles is a harder problem. It's entirely possible that just applying a dark stylesheet to about:blank will solve the problem 99% of the time. But I kind of doubt it. And there will always be the other 1%. It also wouldn't help when loading remote layers when switching tabs.

The better solution to this specific problem is probably to allow extensions to change the default document colors. We have an API to allow them to force the default colors to override page styles[2], but not to allow them to set those colors yet (bug 1459504, bug 1431473).

That would be a broad stroke solution, and would only really work for extensions that want to set a dark theme everywhere. I suppose we could consider another API to allow setting default viewport and tab-loading-blank-content colors based on URL patterns, but I'm not sure how warranted that is.

[1]: https://searchfox.org/mozilla-central/rev/00dd116638001772fe354b081353b73f1cad405d/toolkit/components/extensions/ExtensionContent.jsm#447-459
[2]: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/browserSettings/overrideDocumentColors

Comment 6

[Kris Maglione [:kmag]]

(In reply to Andrew Swan [:aswan] from comment #4)
> If this is the Stylus source code:
> https://github.com/openstyles/stylus
> 
> then the extension appears to inject a content script into every page that
> sends a message to the background page asking what styles to apply.  This is
> always going to be racy (in addition to being needlessly expensive).

Oh. OK, in that case, yeah, we can't really prevent the FOUC.

I'm still not sure how bad the problem is when using static content scripts, though. I suspect we still have flash-of-unstyled-viewport in a lot of cases, even with those. And tab spinner colors are probably not great, either.

Although, now that I think about it, we can probably solve the tab spinner colors automatically by just caching the tab background color in the parent all the time.

Comment 7

[Éter]

Thank you for your responses. I want to add that this problem is not a "flash-of-unstyled-about:blank" because I am using this beauty: https://github.com/overdodactyl/ShadowFox

Also I want to make clear that this problem only appears on some sites (like the examples that I mentioned earlier), and that writing the same exact style in userContent.css resolve it in every case.

Thank you again.

Comment 8

[Éter]

Also, I dont know what is a "flash of unstyled viewport", or what do you mean by "flash of unstyled content", but keep in mind that this problem is happening when using ShadowFox (https://github.com/overdodactyl/ShadowFox), that styles the entire user interface and protected webpages (such as about: pages and addons.mozilla.org) with a beautiful dark style.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Right, userContent.css would never have this problem, because Firefox itself loads it, and loads it early enough that it's ready before we start loading any documents.  It's then applied to documents when they are created, so you can never have a document that it's not applying to.

Comment 10

[Niklas Gollenstede]

I am the developer of reStyle, which is a user style manager compatible with the traditional Stylish styles.
Currently, reStyle uses browser.tabs.insertCSS(..., { cssOrigin: 'user', }) for every style on every content frame.
here is my current take on browser.contentScripts.register():

It would probably be much more efficient to use `browser.contentScripts.register()`. 
(And then maybe even use one contentScript for multiple Styles.) 
See: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/contentScripts/register 
  
  Currently, there are two blocking problems: 
   1.) `script.remove()` does not remove the CSS from loaded tabs. 
       Patches are welcome: https://bugzilla.mozilla.org/show_bug.cgi?id=1423323 
   2.) There is no way to specify the `cssOrigin`, so `@-moz-document` rules won't work (https://bugzilla.mozilla.org/show_bug.cgi?id=1035091). 
       I see two possible solutions: 
        * Request a `cssOrigin` property either on the `contentScriptOptions` or each entry in its `.css`. 
        * Use the content scripts `.matches` option to include it only on the desired domains in the first place. 
          This should again be faster than injecting it everywhere and then filtering through `@document` blocks, 
          but `.matches` only supports match patterns. They can emulate `url()`, `domain()`, and `url-prefix()`, 
          but not `regexp()` includes, which again leaves two options: 
           * Use `.register({ matches: [ ..., ], })` where possible, and the current implementation for `regexp()`. 
           * Request RegExps in `.matches`.

Comment 11

[Éter]

With the help of the Stylus developer, we have found that in fact, as Andrew Swan mentioned, using contentScripts.register on Stylus the bright flashes disappear.

I wish that in the future, contentScripts.register API could be in a better state for user styles extensions to be able to use it without the problems that Niklas Gollenstede commented.

Thank you all for your responses.
Regards.

Comment 12

[Éter]

More information from the Stylus developer:
"So I've used contentScripts.register to get rid of the "racy" asynchronous messaging and provide the style CSS as a variable in our content script synchronously at document_start. The CSS was applied via style DOM elements like previously.

It didn't help. Only direct registration of literal CSS code helps, which means the problem is not the "racy" messaging like that Firefox dev said. [...]

I'm not sure what to do next, as I don't want to use direct registration of CSS due to its lack of stylesheet ordering, which is needed for our LivePreview of styles in the editor and dynamic toggling of styles. So far the only solution is to re-register all styles that have a higher id on each change of the style code. Yuck."

More information in this issue:
https://github.com/openstyles/stylus/issues/287

Comment 13

[David Durst [:ddurst]]

NI aswan for followup on #c12.

Comment 14

[Andrew Swan [:aswan]]

(In reply to Éter from comment #12)

I'm not sure what to do next, as I don't want to use direct registration of
CSS due to its lack of stylesheet ordering, which is needed for our
LivePreview of styles in the editor and dynamic toggling of styles. So far
the only solution is to re-register all styles that have a higher id on each
change of the style code. Yuck."

Does ordering here refer to the equivalent of cssOrigin? Or to ordering individual entries of a given type?

In either case, adding something to the contentScripts.register() api to give extensions finer control over where individual sheets fall in the cascade order sounds reasonable in principle (I don't know off the top of my head how practical the implementation would be)

Comment 15

[David Durst [:ddurst]]

(This bug will be closed at next triage (1 week from today) if there is no response.)

Comment 16

[Boris Zbarsky [:bzbarsky]]

It's not clear what response you're looking for. Comment 14 asks reporter some questions, but reporter is not in a position to provide those answers afaict; the developers of the relevant extensions are. Have we tried contacting them?

Comment 17

[Andrew Swan [:aswan]]

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #16)

It's not clear what response you're looking for. Comment 14 asks reporter some questions, but reporter is not in a position to provide those answers afaict; the developers of the relevant extensions are. Have we tried contacting them?

Whoops, I overlooked that most of comment 14 was quoted, thank you for pointing that out.

Redirecting the needinfo. Jeremy, would adding cssOrigin to contentScripts.register() meet your needs?

Comment 18

[David Durst [:ddurst]]

Closing. cssOrigin referenced in c17 will be filed in another bug.