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

Rustdoc generates illegal "<div>" inside "<pre>" with traits #64371

Closed
kentfredric opened this issue Sep 11, 2019 · 7 comments · Fixed by #95568
Closed

Rustdoc generates illegal "<div>" inside "<pre>" with traits #64371

kentfredric opened this issue Sep 11, 2019 · 7 comments · Fixed by #95568
Labels
C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@kentfredric
Copy link

Similar to the previous bugs I've filed in this regard with illegal html:

In: 1.39.0-nightly (34e82a7 2019-09-10)

The following trait definition:

#[cfg_attr(
    feature = "external_doc",
    doc(include = "CacheFieldCollection.md")
)]
#[cfg_attr(
    not(feature = "external_doc"),
    doc = "An abstact representation of 'a thing' that can expose data \
           about its unit."
)]
pub trait CacheFieldCollection {
    /// Returns a struct containing data recorded for this thing
    fn fields(&self) -> &CacheFieldData;
    /// Returns a timestamp indicating time of last modification/update
    fn mtime(&self) -> &chrono::DateTime<Utc>;
}

Emits invalid html ( hand formatted ):

<div class="docblock type-decl hidden-by-usual-hider">
<pre class='rust trait'>pub trait CacheFieldCollection {
    fn <a href='#tymethod.fields' class='fnname'>fields</a>(&amp;self) -&gt; &amp;<a class="struct" href="../ccache_stats_reader/struct.CacheFieldData.html" title="struct ccache_stats_reader::CacheFieldData">CacheFieldData</a>;
<div class='item-spacer'></div>    fn <a href='#tymethod.mtime' class='fnname'>mtime</a>(&amp;self) -&gt; &amp;<a class="struct" href="https://docs.rs/chrono/latest/chrono/datetime/struct.DateTime.html" title="struct chrono::datetime::DateTime">DateTime</a>&lt;<a class="struct" href="https://docs.rs/chrono/latest/chrono/offset/utc/struct.Utc.html" title="struct chrono::offset::utc::Utc">Utc</a>&gt;;
}</pre></div>

"pre", like headers, only permits child nodes that are "phrasing content", of which, div is excluded.

So the error is from <div class='item-spacer'></div>

htmltidy handles this by prematurely ending the <pre> before the <div>, and then reinserting a <pre> afterwards, breaking layout:

snapshot_20190911_160411

@jonas-schievink jonas-schievink added C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Sep 11, 2019
@hirschenberger
Copy link
Contributor

So, I assume a <span class="item-spacer"></span> would be legal as it is phrasing content.

I can change that?

@nox
Copy link
Contributor

nox commented Sep 14, 2019

Why is there even an element to do spacing, of all things?

@hirschenberger
Copy link
Contributor

I think it makes sense as the non spacing list looks more confusing.

Screenshot_2019-09-15 modbus Client - Rust
Screenshot_2019-09-15 modbus Client - Rust(2)

@nox
Copy link
Contributor

nox commented Sep 15, 2019 via email

@kentfredric
Copy link
Author

Its also rather redundant in the context of a <pre> where all you need to add a line feed is ...:

 print!("\n")

Or equivalent.

I mean, you're already doing horizontal alignment with literal whitespace.

@hirschenberger
Copy link
Contributor

Its also rather redundant in the context of a <pre> where all you need to add a line feed is ...:

 print!("\n")

Or equivalent.

I mean, you're already doing horizontal alignment with literal whitespace.

Yes, adding an extra '\n' instead if the spacer-div adds a slightly bigger vertical space, but looks ok.

@nox
Copy link
Contributor

nox commented Sep 16, 2019

If you really want markup, then IMO each trait item should be in its own li tag. A trait contents are a list of trait items, after all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants