Move scripts on the rustdoc template into head and apply the defer attribute#90872
Conversation
|
r? @ollie27 (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
r? @jsha |
jyn514
added
the
T-rustdoc
jsha
left a comment
jsha
left a comment
There was a problem hiding this comment.
Generally looks good, thanks for picking this up! One small change to request.
There was a problem hiding this comment.
Let's omit the defer here. This doesn't actually load any external JS, and is just a convenient place to hang some data- attributes.
There was a problem hiding this comment.
Oh and there's even a citation from MDN: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/script#attr-defer
defer
Warning: This attribute must not be used if the src attribute is absent (i.e. for inline scripts), in this case it would have no effect.
There was a problem hiding this comment.
Also, storage and crates JS should not get the defer attribute. They are both on the critical path for rendering the page, so we actually want them to block page render, rather than cause a re-style later.
As an example of why this is important, try changing to a dark theme, and then reloading the page. If storage has the defer attribute, you'll see the page first load with light styles and then with dark (assuming your system default theme is light).
There was a problem hiding this comment.
@jsha
Thank you so much for your extremely clear review! I've made changes as you show :)
jsha
added
S-waiting-on-author
jyn514
added
S-waiting-on-review
|
@bors r+ Thanks @ken-matsui ! |
|
📌 Commit 88787a3 has been approved by |
bors
added
S-waiting-on-bors
…plate, r=jsha Move `scripts` on the rustdoc template into `head` and apply the `defer` attribute Closes rust-lang#90719
…plate, r=jsha Move `scripts` on the rustdoc template into `head` and apply the `defer` attribute Closes rust-lang#90719
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (80f5f60): comparison url. Summary: This benchmark run did not return any relevant changes. If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression |
8 participants
Closes #90719