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

Emit more valid HTML from rustdoc #93576

Merged
merged 1 commit into from
Feb 5, 2022
Merged

Conversation

jsha
Copy link
Contributor

@jsha jsha commented Feb 2, 2022

Previously, tidy-html5 (tidy) would complain about a few things in our HTML. The main thing is that <summary> tags can't contain <div>s. That's easily fixed by changing out the <div>s for <span>s with display: block.

However, there's also a rule that <span>s can't contain heading elements. <span> permits only "phrasing content" https://developer.mozilla.org/en-US/docs/Web/HTML/Element/span, and <h3> (and friends) are "Flow content, heading content, palpable content". https://developer.mozilla.org/en-US/docs/Web/HTML/Element/Heading_Elements

We have a wrapping <div> that goes around each <h3>/<h4>, etc. We turn that into a <section> rather than a <span> because <section> permits "flow content". https://developer.mozilla.org/en-US/docs/Web/HTML/Element/section

After this change we get only three warnings from tidy, run on struct.String.html:

line 6 column 10790 - Warning: trimming empty
line 1 column 1118 - Warning: proprietary attribute "disabled"
line 1 column 1193 - Warning: proprietary attribute "disabled"

The empty <span> is a known issue - there's a span in front of the search box to work around a strange Safari issue.

The <link> attributes are the non-default stylesheets. We can probably refactor theme application to avoid using this proprietary "disabled" attribute.

We can suppress those warnings with flags to tidy, and get a run that returns 0 (success):

tidy -o /dev/null -quiet --drop-empty-elements no --warn-proprietary-attributes no build/x86_64-unknown-linux-gnu/doc/std/string/trait.ToString.html 

Note: this requires the latest version of tidy-html5, built from https://github.com/htacg/tidy-html5. Older versions (including the default version on Ubuntu 21.10) think <section> can't occur inside <summary>.

Demo: https://rustdoc.crud.net/jsha/fix-rustdoc-html/std/string/struct.String.html

r? @GuillaumeGomez

Previously, tidy-html5 (`tidy`) would complain about a few things in our
HTML. The main thing is that `<summary>` tags can't contain `<div>`s.
That's easily fixed by changing out the `<div>`s for `<span>`s with
`display: block`.

However, there's also a rule that `<span>`s can't contain heading
elements. `<span>` permits only "phrasing content"
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/span, and
`<h3>` (and friends) are "Flow content, heading content, palpable
content".
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/Heading_Elements

We have a wrapping `<div>` that goes around each `<h3>`/`<h4>`,
etc. We turn that into a `<section>` rather than a `<span>` because
`<section>` permits "flow content".
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/section

After this change we get only three warnings from tidy, run on
struct.String.html:

line 6 column 10790 - Warning: trimming empty <span>
line 1 column 1118 - Warning: <link> proprietary attribute "disabled"
line 1 column 1193 - Warning: <link> proprietary attribute "disabled"

The empty `<span>` is a known issue - there's a span in front of the
search box to work around a strange Safari issue.

The `<link>` attributes are the non-default stylesheets. We can probably
refactor theme application to avoid using this proprietary "disabled"
attribute.
@rust-highfive
Copy link
Collaborator

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez,@Folyd

@rustbot rustbot added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Feb 2, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 2, 2022
@GuillaumeGomez
Copy link
Member

Thanks!

@bors: r+

@bors
Copy link
Contributor

bors commented Feb 4, 2022

📌 Commit 32f6260 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 Feb 4, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 5, 2022
…askrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#90132 (Stabilize `-Z instrument-coverage` as `-C instrument-coverage`)
 - rust-lang#91589 (impl `Arc::unwrap_or_clone`)
 - rust-lang#93495 (kmc-solid: Fix off-by-one error in `SystemTime::now`)
 - rust-lang#93576 (Emit more valid HTML from rustdoc)
 - rust-lang#93608 (Clean up `find_library_crate`)
 - rust-lang#93612 (doc: use U+2212 for minus sign in integer MIN/MAX text)
 - rust-lang#93615 (Fix `isize` optimization in `StableHasher` for big-endian architectures)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3edec80 into rust-lang:master Feb 5, 2022
@rustbot rustbot added this to the 1.60.0 milestone Feb 5, 2022
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.

5 participants