1834786 - Some pages are completely broken with Dark Reader

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[bloonyapprentice]

Attachment: broken webpage.png

Steps to Reproduce

1. Visit https://www.dictionary.com/browse/based
2. Observe that the webpage displays perfectly fine.
3. Install Dark Reader.
4. Refresh the page with Dark Reader on.

Expected Result

The webpage should display perfectly fine with or without Dark Reader.

Actual Result

The webpage is completely unusable.

Regression Range

2023-05-24T17:38:52.703000: DEBUG : Found commit message:
Bug 1834081 - Cache inline stylesheets regardless of shadow dom. r=mstange

Some pages (like Reddit) would have duplicate multi-kilobyte stylesheets
even at the top level.

Differential Revision: https://phabricator.services.mozilla.com/D178548

2023-05-24T17:38:52.703000: DEBUG : Did not find a branch, checking all integration branches
2023-05-24T17:38:52.704000: INFO : The bisection is done.
2023-05-24T17:38:52.706000: INFO : Stopped

Comment 1

[bloonyapprentice]

Attachment: console-errors.txt

This is what the console shows.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1834786 - Don't return cached stylesheets if the sheet principal doesn't match. r=#style,#layout

Otherwise stuff accessing stylesheets from a sandbox and content might
conflict, and we might incorrectly deny giving access to a CSSStyleSheet
rules.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: WIP: Bug 1834786 - Attempt at a test.

It somehow fails with an stack-trace-less exception, needs more
debugging.

Depends on D178971

Comment 4

[Daniel Holbert [:dholbert]]

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

Created attachment 9335725 [details]
WIP: Bug 1834786 - Attempt at a test.

It somehow fails with an stack-trace-less exception, needs more
debugging.

We should be sure to get the fix in before the soft freeze next Thursday; ideally with a few days' wiggle room. If it's possible to complete the test before then, that'd be ideal; but if it's getting tight and the test remains mysterious, maybe it's better to have the fix in earlier and get some nightly validation/testing on the new code before 115 ships to beta, and track finishing the test in a priority followup.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1834081

Comment 6

[tukusejssirs]

I just want to say that I am affected by this bug too and besides dictionary.com, it fails to work on any and all Grafana instances. Here’s a public demo server → https://play.grafana.org/d/000000012/grafana-play-home?orgId=1

Comment 7

[Benjamin Neff [:supertux88]]

I just looked at the regression range above, and I saw that a config option was added for this new behavior, and indeed, switching layout.css.inline-style-caching.always-enabled to false fixes the problem for me and Dark Reader works again. So this works as a workaround until it gets fixed.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/47f0f49b4036
Don't return cached stylesheets if the sheet principal doesn't match. r=dholbert

Comment 9

[tukusejssirs]

Thanks, @supertux88! I can confirm that disabling layout.css.inline-style-caching.always-enabled fixes this issue on FF Nightly 115.0a1 (2023-05-27) (64-bit).

Do I get it right that Dark Reader needs to support inline styles in order to make this work even when layout.css.inline-style-caching.always-enabled is enabled?

Comment 10

[Emilio Cobos Álvarez (:emilio)]

(In reply to tukusejssirs from comment #9)

Do I get it right that Dark Reader needs to support inline styles in order to make this work even when layout.css.inline-style-caching.always-enabled is enabled?

No, this is a Firefox bug. The patch in comment 8 fixes it . I'll work on an automated test for this, but the TLDR is that stylesheets created by extensions and stylesheets created by the website have different access scopes, and we were mixing them up in some cases in a way that caused dark reader to misbehave.

Comment 11

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/47f0f49b4036

Comment 13

[Emilio Cobos Álvarez (:emilio)]

Attachment: WIP: Bug 1834786 - Attempt at a test.

Comment 14

[Emilio Cobos Álvarez (:emilio)]

I tracked down the exception and tried to test this in a different way, however I couldn't get it to fail before patch. Punting on it for now but will get back to it.

Comment 15

[Ardelean Oana, Desktop Test Engineering [:oardelean]]

Reproducible o a 2023-05-27 Nightly build on macOS 12.
Verified as fixed on Firefox 115.0b2(build ID: 20230606180112) and Nightly 116.0a1(build ID: 20230607214358) on macOS 12, Windows 10, Ubuntu 22.

Comment 16

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/4ffac98ebb2a
Backed out changeset 47f0f49b4036 (bug 1834786) from beta, the regressor (bug 1834081) is being backed out