1906793 - Text disappears after trivial devtools tweaks, for testcase that uses a web-font that's blocked by CSP

Status: VERIFIED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Daniel Holbert [:dholbert]]

STR:

1. Load https://hg.mozilla.org/mozilla-central/raw-file/tip/layout/reftests/writing-mode/1083848-1-inline-border.html
2. Right-click the "A B C" text on the first-line and choose "Inspect", to inspect the <div class="test h"> element.
3. In the DevTools Rules inspector for that element, do either one of the following (doesn't matter which):
   (a) Untick the letter-spacing declaration.
   ...Or:
   (b) Click to insert a new declaration and just type color:

ACTUAL RESULTS:
The regular-weight text in that element ("A B C D E" / "U V W X Y Z") all disappears. This is a paint artifact; the text comes back if you close devtools, or force a paint by e.g. trying to select the hidden text.

EXPECTED RESULTS:
No such disappearing text.

NOTE:
I've tried to create a reduced testcase that doesn't involve manual devtools interaction or the hg.mozilla.org-hosted web-font, but haven't been able to do so yet.

Regression range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=2544b23196f95d2acf8dc0e39440ba04fa209278&tochange=fdb157285d27ca4f3a163d1b7dc4bc8ae5ab85df

--> Regression from bug 1851118, which is about font loading.

Note that the test here does use a web-font, but the web font is blocked due to the CSP on hg.mozilla.org; that might be part of the requirement to trigger the bug here, somehow.

Comment 1

[Daniel Holbert [:dholbert]]

Here's a a profile, with the reftest zoomed in 500% to be extra visible in the screenshots track, and using option 3b (typing color:) as the devtools tweak: https://share.firefox.dev/3xOMh9f

Comment 2

[Daniel Holbert [:dholbert]]

Here's the moment where we switch to the "bad" rendering in that profile:
https://share.firefox.dev/3zBcDfx

The content process has two restyles there, both with the following backtrace:

InspectorUtils.parseStyleSheet []
mozilla::dom::InspectorUtils_Binding::parseStyleSheet(JSContext*, unsigned int, JS::Value*) [dom/bindings/InspectorUtilsBinding.cpp]
mozilla::dom::InspectorUtils::ParseStyleSheet(mozilla::dom::GlobalObject&, mozilla::StyleSheet&, nsTSubstring<char> const&, mozilla::ErrorResult&) [layout/inspector/InspectorUtils.cpp]
mozilla::StyleSheet::ReparseSheet(nsTSubstring<char> const&, mozilla::ErrorResult&) [layout/style/StyleSheet.cpp]
mozilla::StyleSheet::RuleRemoved(mozilla::css::Rule&) [layout/style/StyleSheet.cpp]
mozilla::dom::Document::RuleRemoved(mozilla::StyleSheet&, mozilla::css::Rule&) [dom/base/Document.cpp]
mozilla::dom::Document::ApplicableStylesChanged(bool) [dom/base/Document.cpp]
mozilla::dom::Document::MarkUserFontSetDirty() [dom/base/Document.cpp]
mozilla::PresShell::EnsureStyleFlush() [layout/base/PresShellInlines.h]
mozilla::PresShell::SetNeedStyleFlush() [layout/base/PresShellInlines.h]

That backtrace indicates that DevTools is nudging us to reparse the stylesheet, which is triggering a call to MarkUserFontSetDirty which might be involved here, given the test's usage of a (CSP-blocked in this case) web-font, and the regressor's relation to font loading.

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: screencast of bug with attached testcase

See attached self-describing testcase which includes its own CSP, sufficient to trigger the bug (when served from a web server at least, e.g. directly viewing the bugzilla attachment).

As in comment 0, "Actual results" are that the text disappears, when it should not.

Comment 5

[Daniel Holbert [:dholbert]]

(side note, to avoid confusion -- the screencast does show font-devtools showing "DejaVu Serif" as the font used to render the text, which sounds sort-of like the web-font that the testcase attempts to use (which is another DejaVu variant that we have in mozilla-central and use for reftests). But that's in fact just coincidence; DejaVu Serif is just my default system font. The font URL that's referenced in the testcase is indeed blocked by the CSP (and isn't pointing to any valid resource anyway).)

Comment 6

[Daniel Holbert [:dholbert]]

emilio or jfkthame, maybe you could take a look here? Not sure if real web content can trigger this invalidation bug, but it seems conceivable that there might be similar web-exposed codepaths.

(As a shot in the dark, I'm guessing that after the invalidation, maybe we try to repaint the text before we're sure about what font we can use to render it, or something along those lines...?)

Comment 7

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

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

Comment 8

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1906793 - Fix invalidation of sync-failing font loads. r=jfkthame,dholbert,#layout

I haven't been able to construct a test-case without InspectorUtils
either. Tried to construct a test-case with InspectorUtils, but that
doesn't work either because the DOM mutation is enough to schedule a
style flush (for invalidation) which ends up triggering the update.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c934ebb0f91c
Fix invalidation of sync-failing font loads. r=jfkthame,layout-reviewers

Comment 10

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/c934ebb0f91c

Comment 11

[Dana Lucaci, Desktop QA]

Reproduced the issue with Firefox 129.0a1 (2024-07-08) on macOS 14.
The issue is verified fixed with Firefox 131.0a1 (20240807215007) and Firefox 130.0b2 (20240807091924) onWindows 10, macOS 14 and Ubuntu 22.