-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Prevent usage of .stab elements to create scrollable areas in doc blocks #102100
Conversation
Some changes occurred in HTML/CSS/JS. cc @GuillaumeGomez, @Folyd, @jsha |
ffeabd0
to
7495e0d
Compare
// Checking that there is no scrollable content. | ||
assert-property: ( | ||
".top-doc .docblock p:nth-of-type(1)", | ||
{"scrollHeight": "120", "clientHeight": "120", "scrollWidth": "502", "clientWidth": "502"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of hard coding the number 502, would it be better to create a fourth paragraph, with no stab tags in it, and use compare-elements-property to assert that they all have the same scroll width?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That wouldn't be "accurate" as for whatever reasons the size could be different. What I'm currently working on is adding support for variables in browser-ui-test
so we can instead directly store either scroll*
or client*
and then compare to the other ones, so no need for raw numbers like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created the issue so you can have an idea of what it will look like: GuillaumeGomez/browser-UI-test#351
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, that makes sense.
7495e0d
to
6069f71
Compare
Fixed the formatting and the invalid CSS selector. |
Ah the CI finally restarted. |
CI passed \o/ |
@bors r+ rollup |
Rollup of 12 pull requests Successful merges: - rust-lang#101952 (Avoid panicking on missing fallback) - rust-lang#102030 (Don't crate-locally reexport walk functions in tidy) - rust-lang#102032 (Adding ignore fuchsia tests for signal interpretation cases) - rust-lang#102033 (Adding needs-unwind to nicer-assert-messages compiler ui tests) - rust-lang#102054 (Unify "all items" page's sidebar with other pages) - rust-lang#102071 (Adding needs-unwind for tests testing memory size of Futures/Closures) - rust-lang#102073 (Adding ignore fuchsia tests for execvp) - rust-lang#102075 (rustdoc: remove no-op CSS `.content > .methods > .method`) - rust-lang#102079 (Update books) - rust-lang#102084 (Adding needs-unwind for test using panic::catch_unwind) - rust-lang#102100 (Prevent usage of .stab elements to create scrollable areas in doc blocks) - rust-lang#102102 (Add doc aliases on Sized trait) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Fixes #101874.
You can test it online here.
r? @notriddle