Bug 181374 - font-display:fallback can cause a visual flash (which is supposed to be impossible)
Summary: font-display:fallback can cause a visual flash (which is supposed to be impos...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Text (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-01-08 00:23 PST by Myles C. Maxfield
Modified: 2018-01-09 16:45 PST (History)
4 users (show)

See Also:


Attachments
Repro (needs external font file and slow server) (2.14 KB, text/html)
2018-01-08 00:49 PST, Myles C. Maxfield
no flags Details
Python Server (906 bytes, text/x-python-script)
2018-01-08 00:52 PST, Myles C. Maxfield
no flags Details
Patch (32.13 KB, patch)
2018-01-08 18:03 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (32.91 KB, patch)
2018-01-08 18:52 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2018-01-08 00:23:06 PST
FontCascadeFonts::primaryFont() specifically hardcodes the first FontRange inside the FontCascade ("realizeFallbackRangesAt(description, 0)"), which could be loading. If it's in the middle of loading, this will result in the interstitial font (Times). All the other codepaths which call realizeFallbackRangesAt() will detect that it returned the interstitial font and keep iterating through the font-family list, but this codepath doesn't. This means that the metrics for the primary font (which is used for things like line-height) may disagree with all the rendered characters.

This is particularly problematic if the loading font transitions to the failed state, which is how font-display:fallback is implemented. If this situation, realizeNextFallback() will keep iterating past the failed font-family item, which means that the primary font will change. This can change things like line-height, and therefore cause a visual flash (which is contrary to the whole reason font-display:fallback exists in the first place).
Comment 1 Myles C. Maxfield 2018-01-08 00:25:41 PST
One solution would be to add a loop inside FontCascadeFonts::primaryFont() to skip over interstitial fonts (the same way all the other callers of realizeFallbackRangesAt() work). Another solution would be to add a new state to CSSFontFace which wouldn't be treated like failure. Both of these solutions are pretty scary changes.
Comment 2 Myles C. Maxfield 2018-01-08 00:49:40 PST
Created attachment 330669 [details]
Repro (needs external font file and slow server)

Repro attached. Requires an external font file and a slow (longer than 3 seconds) server when serving the font file.
Comment 3 Myles C. Maxfield 2018-01-08 00:50:11 PST
The repro shows how the text moves because the line height changes after 3 seconds because the font-face block gets marked as failed.
Comment 4 Myles C. Maxfield 2018-01-08 00:52:32 PST
Created attachment 330670 [details]
Python Server
Comment 5 Myles C. Maxfield 2018-01-08 18:03:53 PST
Created attachment 330770 [details]
Patch
Comment 6 Myles C. Maxfield 2018-01-08 18:52:24 PST
Created attachment 330776 [details]
Patch
Comment 7 WebKit Commit Bot 2018-01-09 16:44:53 PST
Comment on attachment 330776 [details]
Patch

Clearing flags on attachment: 330776

Committed r226668: <https://trac.webkit.org/changeset/226668>
Comment 8 WebKit Commit Bot 2018-01-09 16:44:55 PST
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2018-01-09 16:45:33 PST
<rdar://problem/36390845>