rustdoc: use CSS containment to speed up render #102253

jsha · 2022-09-25T05:51:26Z

https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Containment

This affected layout a little and required adjustments to the CSS to keep spacing the same. In particular, the margins of adjacent items usually overlap with each other. However, when an item has contain: layout, any margins of child nodes push out the size of the item itself. This was making spacing between items a little too big. To solve that, I removed margins in some places: in particular for certain classes that often occur at the end of a details.rustdoc-toggle block, I removed their bottom margin. Generally, the margins provided by the next item down are sufficient.

Also remove an unnecessary margin-top on .code-header.

In particular this helps with the problem that rustdoc in some situations can generate giant HTML pages, which can crash a Chrome tab on typical modern hardware, for instance: https://docs.rs/iced-x86/1.16.0/iced_x86/code_asm/struct.CodeAssembler.html (26MB, 409k DOM nodes). This doesn't, of course, universally solve the problem, but it pushes out the boundary of the largest page rustdoc can produce without crashing a browser tab.

Demos:

https://rustdoc.crud.net/jsha/css-contain/std/string/struct.String.html
(warning: giant page, may crash a browser tab) https://rustdoc.crud.net/jsha/css-contain-icedx86/iced_x86/code_asm/struct.CodeAssembler.html

r? @notriddle

rustbot · 2022-09-25T05:51:29Z

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @Folyd, @jsha

GuillaumeGomez · 2022-09-25T09:23:31Z

You're doing two different things in here: reducing margins and speeding up page load. The second one is a great improvement, the first one I'm not sure to understand why it was done. It makes the CSS a bit more complicated so I assume that there is a good reason behind it. Can you provide more information about it please?

notriddle · 2022-09-25T16:40:46Z

The appearance of the page shouldn’t change. The reduced margins are supposed to make up for the way that contain: layout turns off margin collapsing.

GuillaumeGomez · 2022-09-25T17:46:25Z

That seems quite easy to break then. :-/

notriddle · 2022-09-25T18:03:51Z

Yeah, we need to fix it so that rustdoc doesn’t rely on margin collapsing so much.

bors · 2022-09-26T06:18:55Z

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

notriddle · 2022-09-26T20:30:57Z

tl;dr: if you update the sidebar-mobile-scroll.goml test case to fix the slightly changed numbers, and get rid of the nav.sidebar { contain } rule to fix the sidebar-source-code-display.goml test case, and fix the merge conflict, then r=me.

`sidebar-mobile-scroll.goml`

[ERROR] (line 9) Error: Evaluation failed: The following errors happened: [Expected `643` for property `pageYOffset`, found `645`]: for command `assert-window-property: {"pageYOffset": "643"}`
[ERROR] (line 24) Error: Evaluation failed: The following errors happened: [Expected `643` for property `pageYOffset`, found `645`]: for command `assert-window-property: {"pageYOffset": "643"}`
[ERROR] (line 31) Error: Evaluation failed: The following errors happened: [Expected `643` for property `pageYOffset`, found `645`]: for command `assert-window-property: {"pageYOffset": "643"}`

Your version

Current nightly

It looks like, in the current nightly, there's a margin-top set on the header, and it's collapsing with the margin-bottom on the toggle. In your version, however, they don't collapse.

If the change is this small, then I don't really care. Just update the test case and move on. It's literally two pixels difference, it looks fine either way, and while you are adding a bit more space to the page, it's spacing added to the global sections, which aren't very numerous.

`sidebar-source-code-display.goml`

[ERROR] (line 230) Error: The following CSS properties still don't match: [width: (`0px` != `500px`)]: for command `wait-for-css: (".sidebar", {"width": "500px"})`

This one's a bit more concerning. Is the nav.sidebar { contain } rule actually necessary to see the performance improvements in Chrome? If not, this can probably be done in another PR.

https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Containment This affected layout a little and required adjustments to the CSS to keep spacing the same. In particular, the margins of adjacent items usually overlap with each other. However, when an item has contain: layout, any margins of child nodes push out the size of the item itself. This was making spacing between items a little too big. To solve that, I removed margins in some places: in particular for certain classes that often occur at the end of a `details.rustdoc-toggle` block, I removed their bottom margin. Generally, the margins provided by the next item down are sufficient. Also remove an unnecessary margin-top on .code-header.

notriddle · 2022-09-27T17:21:42Z

src/librustdoc/html/static/css/rustdoc.css

 	margin-bottom: 2em;
 }

-.implementors-toggle[open]  {
+.implementors-toggle[open]:not(:last-child) {
 	margin-bottom: 2em;


This 2em margin works out to be 32px.

This means that, by disabling it on the last child, you're reducing the margin on the bottom of a list of implementors in this scenario (on current nightly, it'll be 32px, while on your branch, it's now 25px, because that's the top margin of the header).

src/test/rustdoc-gui/sidebar-mobile-scroll.goml

Co-authored-by: Michael Howell <michael@notriddle.com>

notriddle · 2022-09-27T18:27:14Z

@bors r+ rollup

bors · 2022-09-27T18:27:16Z

📌 Commit b5b77a2 has been approved by notriddle

It is now in the queue for this repository.

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#101555 (Stabilize `#![feature(mixed_integer_ops)]`) - rust-lang#102253 (rustdoc: use CSS containment to speed up render) - rust-lang#102281 (make invalid_value lint a bit smarter around enums) - rust-lang#102284 (Structured suggestion for missing `mut`/`const` in raw pointer) - rust-lang#102330 (rustdoc: remove no-op CSS `.srclink { font-weight; font-size }`) - rust-lang#102337 (Avoid LLVM-deprecated `Optional::hasValue`) - rust-lang#102356 (session: remove now-unnecessary lint `#[allow]`s) - rust-lang#102367 (rustdoc: remove redundant `#help-button` CSS) - rust-lang#102369 (Fix search result colors) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rust-highfive assigned notriddle Sep 25, 2022

rustbot added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Sep 25, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 25, 2022

jsha force-pushed the css-contain branch from ae38cd5 to 410ed49 Compare September 25, 2022 19:52

This comment has been minimized.

Sign in to view

jsha force-pushed the css-contain branch from 410ed49 to b026853 Compare September 27, 2022 16:15

jsha force-pushed the css-contain branch from b026853 to 8846c08 Compare September 27, 2022 16:21

This comment has been minimized.

Sign in to view

notriddle reviewed Sep 27, 2022

View reviewed changes

src/test/rustdoc-gui/sidebar-mobile-scroll.goml Outdated Show resolved Hide resolved

notriddle reviewed Sep 27, 2022

View reviewed changes

src/test/rustdoc-gui/sidebar-mobile-scroll.goml Outdated Show resolved Hide resolved

notriddle reviewed Sep 27, 2022

View reviewed changes

src/test/rustdoc-gui/sidebar-mobile-scroll.goml Outdated Show resolved Hide resolved

Update src/test/rustdoc-gui/sidebar-mobile-scroll.goml

b5b77a2

Co-authored-by: Michael Howell <michael@notriddle.com>

jsha force-pushed the css-contain branch from 46fef92 to b5b77a2 Compare September 27, 2022 17:52

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 Sep 27, 2022

matthiaskrgr mentioned this pull request Sep 27, 2022

Rollup of 9 pull requests #102377

Merged

clubby789 mentioned this pull request Sep 27, 2022

code_asm: CodeAssembler docs.rs page causes Chrome page crash icedland/iced#351

Closed

bors merged commit 4cef648 into rust-lang:master Sep 28, 2022

rustbot added this to the 1.66.0 milestone Sep 28, 2022

Alexendoo mentioned this pull request Oct 2, 2022

Notable traits tooltip is laid out underneath subsequent methods #102576

Closed

cynecx mentioned this pull request Dec 11, 2022

code area isn't scrollable on mobile (rustdoc) #105580

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustdoc: use CSS containment to speed up render #102253

rustdoc: use CSS containment to speed up render #102253

jsha commented Sep 25, 2022 •

edited

Loading

rustbot commented Sep 25, 2022

GuillaumeGomez commented Sep 25, 2022

notriddle commented Sep 25, 2022 •

edited

Loading

GuillaumeGomez commented Sep 25, 2022

notriddle commented Sep 25, 2022

bors commented Sep 26, 2022

This comment has been minimized.

notriddle commented Sep 26, 2022

This comment has been minimized.

notriddle Sep 27, 2022

notriddle commented Sep 27, 2022

bors commented Sep 27, 2022

rustdoc: use CSS containment to speed up render #102253

rustdoc: use CSS containment to speed up render #102253

Conversation

jsha commented Sep 25, 2022 • edited Loading

rustbot commented Sep 25, 2022

GuillaumeGomez commented Sep 25, 2022

notriddle commented Sep 25, 2022 • edited Loading

GuillaumeGomez commented Sep 25, 2022

notriddle commented Sep 25, 2022

bors commented Sep 26, 2022

This comment has been minimized.

notriddle commented Sep 26, 2022

sidebar-mobile-scroll.goml

Your version

Current nightly

sidebar-source-code-display.goml

This comment has been minimized.

notriddle Sep 27, 2022

Choose a reason for hiding this comment

notriddle commented Sep 27, 2022

bors commented Sep 27, 2022

jsha commented Sep 25, 2022 •

edited

Loading

notriddle commented Sep 25, 2022 •

edited

Loading

`sidebar-mobile-scroll.goml`

`sidebar-source-code-display.goml`