Open Bug 1844683 Opened 1 year ago Updated 1 year ago

Different behavior than other browsers when detecting cyclic imports.

Categories

(Core :: CSS Parsing and Computation, defect, P3)

Firefox 115
defect

Tracking

()

UNCONFIRMED

People

(Reporter: romainmenke, Unassigned)

References

Details

Steps to reproduce:

see : https://github.com/romainmenke/firefox-css-cyclic-imports-bug-001
This contains a full repro.


Steps :

  • run Firefox through puppeteer
  • load a page with cyclic CSS imports

The result seems to be different with headless Firefox than running Firefox regularly.
The tests try to display a green box.

Actual results:

When visiting a page in regular Firefox the box is green.
When running headless with puppeteer the box is reported as red.

Expected results:

The box should have been green consistently in all cases.

The Bugbug bot thinks this bug should belong to the 'Core::CSS Parsing and Computation' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Component: Untriaged → CSS Parsing and Computation
Product: Firefox → Core

The severity field is not set for this bug.
:emilio, could you have a look please?

For more information, please visit BugBot documentation.

Flags: needinfo?(emilio)

Do you know of any setting that's specific to puppeteer which could affect this?

Severity: -- → S3
Flags: needinfo?(emilio) → needinfo?(hskupin)
Priority: -- → P3

There is actually no need to use Puppeteer. This can also be seen with Marionette enabled. Here the steps to reproduce:

  1. Clone https://github.com/romainmenke/firefox-css-cyclic-imports-bug-001
  2. Start Firefox with ./mach run
  3. Open firefox-css-cyclic-imports-bug-001/tests/cycles/006/index.html and observe that the square is green
  4. Start Firefox with ./mach run --marionette
  5. Open firefox-css-cyclic-imports-bug-001/tests/cycles/006/index.html and observe that the square is red
  6. Repeat step 2 and 3 and observe that the square is green again
Flags: needinfo?(hskupin)

The problematic pref here is security.fileuri.strict_origin_policy, which for security reasons we set to false for all of the remote protocols.

The test should probably use a web server for serving the related files.

When using any web server the box is indeed always red.
I've updated my reproduction with this new info.

There is also an issue about this on csswg : https://github.com/w3c/csswg-drafts/issues/9171

Emilio could you please have a look given that this is now a manual testcase and the Remote Protocol and Marionette are both not involved?

Flags: needinfo?(emilio)

So let's center in the web server case, since in the other case the issue is a CORS error. Let's check 006. The tree is:

b.css #1
 |_green.css
 |_a.css
    |_red.css
    |_b.css #2 - Has to be clearly cut-off, it's loaded from b.css
c.css
 |_a.css
    |_red.css
    |_b.css #3 - Firefox is cutting off b.css here
      |_green.css
      |_a.css # Has to be clearly cut-off, it's loaded from a.css

So I think there are two main behavior differences.

The first is that, for our import cycle detection, we look at other parents of the same sheet. This is done here, and turns out it comes all the way from bug 645998.
Our behavior isn't the most intuitive, and I'm not sure why looking only at ancestors would make the test-case from that bug break?

The other, more important one, is that coalesce stylesheet loads more aggressively than other browsers (and I think we want to keep doing that). So the b.css load in #3 is just re-using the first b.css load, and thus not blocking a.css.

So for Firefox the tree ends up looking:

b.css #1
 |_green.css
 |_a.css
    |_red.css
c.css
 |_a.css
    |_red.css
    |_b.css (just reuses #1, no load is performed)
      |_green.css
      |_a.css
         |_red.css

We could disable the cache re-use for any stylesheet that has a cyclic import, but honestly it seems rather edge-case-y and a bit unfortunate.

Flags: needinfo?(emilio)
See Also: → 645998
Summary: Cyclic stylesheets aren't correctly applied when running headless → Different behavior than other browsers when detecting cyclic imports.

(In reply to Emilio Cobos Álvarez (:emilio) from comment #9)

So let's center in the web server case, since in the other case the issue is a CORS error.

Err, scratch this. The issue here is that we don't manage to reuse the load because each file:// stylesheet ends up having a different principal (read: security boundary). So which stylesheet triggered the load ends up being part of the cache key (probably unnecessarily).

You need to log in before you can comment on or make changes to this bug.