Optional too optional, apparently

[workingjubilee]

Local testing did not reveal that "optional" is incredibly eagerly skipped
if it involves network transit, so successive page loads didn't actually
load the fonts. "fallback" behaved as-predicted (success on next page
load). Some styles set to "swap" to guarantee better readability.

In a word: "Oops."
Partially backs off the change in #72092

[Nemo157]

FWIW optional at least appears to work as intended in Safari (on non-cellular connection (for the 0.0001% of us that use it)).

[Nemo157]

It looks like it does work in Firefox as well, https://docs.rs/femme/2.1.0/femme/ at least loads 4 of the 7 fonts (presumably only the ones that are used) and they are used on the second viewing of the page (as long as devtools is closed, with devtools open it seems to timeout loading the fonts for some reason and uses the fallback).

[steveklabnik]

r? @GuillaumeGomez

[workingjubilee]

Oh hm. It feels really wonky while I browse https://doc.rust-lang.org/nightly/std/ but if devtools affects it somehow, then maybe my viewing information was "corrupted" in a way. That or my browser just decided I had a REALLY awful connection, and it worked As Intended.

OK, formally noting I don't feel confident this entire patch is an essential change, I was just surprised by some of the initial results.

[Nemo157]

Ok, interesting, clicking on the arch self-link on https://doc.rust-lang.org/nightly/core/arch/index.html over and over again (after clearing the cache the first time) it's ~50/50 whether it will use the webfonts or not. It might be affected by page size, larger pages take longer to load so hit whatever the limit is at which the browser gives up and goes to the fallback. My previous testing was all on relatively small pages on docs.rs.

[GuillaumeGomez]

I just tested locally too, didn't expect much changes on externally hosted pages... So what do you want to do here @workingjubilee ?

[workingjubilee]

Hm. So, thank you Nemo. With that information, I now believe rustdoc can do better than this PR.

However, having given it that bit of thought, I still (once more?) believe this PR is correct because font-display is basically choosing presentation vs. performance, and the previous PR was made with an assumption that optional would err slightly more in favor of presentation than it actually does. As this PR errs more in favor of presentation, this PR is closer to the one I would have made if I had known what I know now.

[bors]

☔ The latest upstream changes (presumably #72639) made this pull request unmergeable. Please resolve the merge conflicts.

[workingjubilee]

tl;dr: I made a few more changes that should correctly minimize any style impact while still collecting low-hanging performance gains. It can be driven much further, but not with just this change.

Since this got blocked due to the merge conflict, and given that I mentioned "rustdoc can do better, probably", I did a bit more research and turned up more detail in conversations like these:
w3c/csswg-drafts#4108
https://bugs.chromium.org/p/chromium/issues/detail?id=1040632
https://bit.ly/36E8UKB
google/fonts#2315
https://bugzilla.mozilla.org/show_bug.cgi?id=1486158

It seems while the font WD has been around for Some Time Now and is mostly implemented, optional in particular is a detail that is more in-flux than I thought it was. Very notably https://discourse.wicg.io/t/support-async-loading-stylesheets-with-low-priority/3760 discusses allowing explicitly the exact declared intent I Ihought I was getting with a different combination of HTML/CSS.

Reviewing the style cascade, however, testing with more configurations, and reading the finalized Font Level 3 specs, browsers implement a "fallback to nearest font, else synthesize style" behavior if a font is missing for a given weight. So, I think fallback on body text is also somewhat more incorrect as a choice given rustdoc's styling concerns, and instead swap should be preferred on main body text, with fallback or optional on "secondary" webfont styles. Given that optional has a hard-to-predict behavior, and there are murmurs of a possible near-future reworking of the CSS, I think fallback is preferred for now, as it is more forward-resilient to editing.

[JohnCSimon]

Ping from triage:
@workingjubilee is this PR waiting on a review?

[workingjubilee]

@JohnCSimon Yes, sorry, did I miss something?

[workingjubilee]

Ah, I suppose screenshots might be good.
This is the before:

This is a hypothetical worst-case after, if everything fails to download until after the time limit, some degradation due to the browser inferring styles but we get a Pretty Correct look, a little odd if you're comparing them right next to each other but nothing shockingly off. This shouldn't happen, though, because the suggested time limit is extremely generous.

Everything will continue to load and the caveats in the prior PR about how it shouldn't be as much of a problem while navigating the site on successive clicks should actually apply this time.

Initial render depends a lot on default user fonts so I'm not including it. It might be good to set a "system font" default if we want to improve style adherence, though it might be good to reassess font selection in general. Rendering performance could be significantly improved by simply using webfonts more judiciously.

[Elinvynia]

@workingjubilee If something is taking a while, feel free to contact the triage WG in the future :) This happened to land on my day of triaging and I've changed the label accordingly.

[jyn514]

@GuillaumeGomez I think this is waiting on your review.

[GuillaumeGomez]

I'm still not sure this is a good idea to do this actually...

[workingjubilee]

...Okay.