1620359 - Viewport units not correctly updated if a following font-face load doesn't cause a restyle.

Status: RESOLVED FIXED

(Core :: Layout, defect, P3)

Description

[beretta.christian2602]

Attachment: view of the problem

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:74.0) Gecko/20100101 Firefox/74.0

Steps to reproduce:

Using an external stylesheet with a font-face declaration in the header of an .HTML file.
Then I use Viewport to define size of a DIV in the CSS.

Actual results:

During the viewing of that on your browser (both mozilla firefox standard and firefox developer edition) and after rescaling the window a white box appear to fill the gap

Expected results:

After resizing of the window the won't be no white box, so the page automatically adjust.

Comment 1

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

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

can you attach or link to the test case so we can look into it? thanks.

Comment 3

[beretta.christian2602]

https://codepen.io/chri-b/pen/abOLygX
Please notice that in the header there is Fontawesome script kit causing the issue.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: Reduced test-case.

Thanks! I could not repro with that example, but thanks to that I could build a test-case that reproduces it consistently for me.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1620359 - Don't clear the "uses viewport units" bit when a font that doesn't cause a style change loads. r=jfkthame,#style

This is probably an old-ish bug made more frequent by the font loading
optimizations.

PostRebuildAllStyleData is a bit of a misnomer, but was always calling
ClearCachedData() on the style set, even if we weren't guaranteed to restyle
every element.

This means both wasted work and correctness issues (as the "uses <rare-feature>"
bits are cleared during this call, on the assumption that we'll then visit all
elements and that'd recompute it properly).

For now, unify a bit the different code paths and only clear these bits if we're
guaranteed to restyle all elements.

I should rename this to something better in a follow-up, and ideally also
decouple the ClearCachedData() calls a bit...

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d2a5c9e9e156
Don't clear the "uses viewport units" bit when a font that doesn't cause a style change loads. r=jfkthame

Comment 7

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/22123 for changes under testing/web-platform/tests

Comment 8

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Can't merge web-platform-tests PR due to failing upstream checks:
Github PR https://github.com/web-platform-tests/wpt/pull/22123
* Community-TC (pull_request) (https://community-tc.services.mozilla.com/tasks/groups/SEb1p15UTaG-f2UM-Am2dw)

Comment 9

[Cristian Brindusan [:cbrindusan]]

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

Comment 10

[Emilio Cobos Álvarez (:emilio)]

(In reply to Web Platform Test Sync Bot (Matrix: #interop:mozilla.org) from comment #8)

Can't merge web-platform-tests PR due to failing upstream checks:
Github PR https://github.com/web-platform-tests/wpt/pull/22123

• Community-TC (pull_request)
  (https://community-tc.services.mozilla.com/tasks/groups/SEb1p15UTaG-f2UM-
  Am2dw)

I think this is because my test passed one out of ten times without the patch applied... James, is that right? If so, maybe admin-merge?

Maybe we shouldn't consider the firefox-stability task to block landing Gecko patches?

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Err, see above ^.

Comment 12

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 13

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot