Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unblock font loading in rustdoc.css #72092

Merged
merged 1 commit into from
May 24, 2020
Merged

Conversation

workingjubilee
Copy link
Member

rustdoc's font loading defaults to "auto", so browsers may block render.
But rustdoc's case prefers a faster TTI for scrolling, this means the
strictest font-display in use should be "swap". rustdoc's fonts do provide
notable legibility improvements but first-time users will have little trouble
reading without. This means "optional" is preferred.

The one exception is Source Serif Pro: it's a big difference for body text, so
"fallback" is preferred over "optional" to cause a (tiny) block.

This follows up on (but does not resolve) #20962, taking PageSpeed Insight's rec fairly directly. Supporting reading material: https://drafts.csswg.org/css-fonts-4/#font-display-desc

rustdoc's font loading defaults to "auto", so browsers may block render.
But rustdoc's case prefers a faster TTI for scrolling, this means the
strictest font-display in use should be "swap". rustdoc's fonts do provide
notable legibility improvements but first-time users will have little trouble
reading without. This means "optional" is preferred.

The one exception is Source Serif Pro: it's a big difference for body text, so
"fallback" is preferred over "optional" to cause a (tiny) block.
@GuillaumeGomez
Copy link
Member

Thanks for the PR! Looks good to me, but before confirming, just a few questions: it first renders the page with the default system font I assume and once the font is loaded, it makes the switch, right? In my low level of understanding, it seems like it actually performs more operations and should be slower, but just to confirm: it's faster this way?

If "yes" is the answer to both questions, then it's good for me! :)

@ghost
Copy link

ghost commented May 11, 2020

The time till you first see text is shorter.
So it feels like the page loads faster, which is what we want.

"optional" lets the user-agent decide if downloading the font is even necessary. Otherwise "swap" would be an alternative.

@workingjubilee
Copy link
Member Author

workingjubilee commented May 12, 2020

Yes† and Yes.
† within sane limits

According to the draft spec and other docs I can find, it appears that most browsers implement a default behavior of beginning to render text anyways (invisibly) and then rerendering the text with the appropriate font once the font is loaded. So browsers just do a lot of ops anyways. These changes speed things up by allowing the browser to not render text twice if the fonts cannot be obtained for that initial rendering pass.

For fallback, if the fonts arrive within greater than 100ms but less than 3s the text is rerendered. This is the recommended behavior for getting body text to load fast (good) and try to guarantee it renders correctly i.e. with the desired font (good) but without mid-reading rerenders (bad). For other pieces of text, both fallback and optional seemed appropriate, so I chose optional, which allows (as mentioned) the choice of just not loading the fonts if they don't immediately arrive.

I would not recommend swap for the main body text. For other text, it risks less of a jarring effect.

On the second visit to a rustdoc page, however, the fonts should already be in cache, so they should load and render instantly. Unless the user is somehow accessing rustdoc webpages on an electronic potato that somehow has a browser installed, in which case the same logic applies.

@crlf0710 crlf0710 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels May 20, 2020
@GuillaumeGomez
Copy link
Member

Fine by me then, thanks!

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented May 23, 2020

📌 Commit 510fce1 has been approved by GuillaumeGomez

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 23, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 23, 2020
…Gomez

Unblock font loading in rustdoc.css

rustdoc's font loading defaults to "auto", so browsers may block render.
But rustdoc's case prefers a faster TTI for scrolling, this means the
strictest font-display in use should be "swap". rustdoc's fonts do provide
notable legibility improvements but first-time users will have little trouble
reading without. This means "optional" is preferred.

The one exception is Source Serif Pro: it's a big difference for body text, so
"fallback" is preferred over "optional" to cause a (tiny) block.

This follows up on (but does not resolve) rust-lang#20962, taking PageSpeed Insight's rec fairly directly. Supporting reading material: https://drafts.csswg.org/css-fonts-4/#font-display-desc
bors added a commit to rust-lang-ci/rust that referenced this pull request May 24, 2020
Rollup of 5 pull requests

Successful merges:

 - rust-lang#71618 (Preserve substitutions when making trait obligations for suggestions)
 - rust-lang#72092 (Unblock font loading in rustdoc.css)
 - rust-lang#72400 (Add missing ASM arena declarations to librustc_middle)
 - rust-lang#72489 (Fix ice-72487)
 - rust-lang#72502 (fix discriminant type in generator transform)

Failed merges:

r? @ghost
@bors bors merged commit 238a0ce into rust-lang:master May 24, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 27, 2020
…changes, r=Dylan-DPC

Remove font-display settings

Since for the moment, the result isn't as expected since rust-lang#72092 when not using docs locally, let's revert them.

r? @Dylan-DPC
@workingjubilee workingjubilee deleted the patch-2 branch April 23, 2021 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants