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 page styles are broken with Waterfox #75011

Closed
ensc opened this issue Aug 1, 2020 · 14 comments
Closed

rustdoc page styles are broken with Waterfox #75011

ensc opened this issue Aug 1, 2020 · 14 comments
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

@ensc
Copy link

ensc commented Aug 1, 2020

https://docs.rs does not work with Waterfox because it relies on the non-standard disabled="" Attribute in the theme <link> tags:

      <link href="/light-20200726-1.47.0-nightly-6c8927b0c.css" id="themeStyle" rel="stylesheet" type="text/css">
      <link disabled="" href="/dark-20200726-1.47.0-nightly-6c8927b0c.css" rel="stylesheet" type="text/css">
      <link disabled="" href="/ayu-20200726-1.47.0-nightly-6c8927b0c.css" rel="stylesheet" type="text/css">

Without "disabled" support (which is the case with Waterfox), all three links will be enabled and will make the (imo unreadable) "ayu" the active one.

Accordingly https://developer.mozilla.org/en-US/docs/Web/HTML/Element/link this attribute requires very recent browsers and is marked as "Deprecated: do not use for new websites"

@jyn514 jyn514 transferred this issue from rust-lang/docs.rs Aug 1, 2020
@jyn514 jyn514 added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Aug 1, 2020
@jyn514
Copy link
Member

jyn514 commented Aug 1, 2020

This is a rustdoc issue, not docs.rs. I transferred to rust-lang/rust.

@jyn514
Copy link
Member

jyn514 commented Aug 1, 2020

Can you give an example page where this happens?

@jyn514 jyn514 changed the title Broken with Waterfox rustdoc page styles are broken with Waterfox Aug 1, 2020
@jonas-schievink jonas-schievink added the C-bug Category: This is a bug. label Aug 1, 2020
@ensc
Copy link
Author

ensc commented Aug 1, 2020

It happens with every crate on https://docs.rs E.g. https://docs.rs/griddle/0.3.1/griddle/ (randomly selected by "I'm feeling lucky")

@jyn514
Copy link
Member

jyn514 commented Aug 1, 2020

I don't see it on https://docs.rs/postgres/0.17.0-alpha.2/postgres/. So it looks like it might have been introduced as part of the Ayu theme, cc @Cldfire

@jyn514
Copy link
Member

jyn514 commented Aug 1, 2020

Would the fix be as simple as changing disabled="" to disabled?

@jyn514
Copy link
Member

jyn514 commented Aug 1, 2020

Hmm, this looks like the relevant code, but it uses disabled, not disabled="".

style_files = style_files
.iter()
.filter_map(|t| {
if let Some(stem) = t.path.file_stem() { Some((stem, t.disabled)) } else { None }
})
.filter_map(|t| {
if let Some(path) = t.0.to_str() { Some((path, t.1)) } else { None }
})
.map(|t| format!(
r#"<link rel="stylesheet" type="text/css" href="{}.css" {} {}>"#,
Escape(&format!("{}{}{}", static_root_path, t.0, page.resource_suffix)),
if t.1 { "disabled" } else { "" },
if t.0 == "light" { "id=\"themeStyle\"" } else { "" }
))
.collect::<String>(),

@ensc
Copy link
Author

ensc commented Aug 1, 2020

@jyn514 yes; the postgres link is ok (seems to be generated by rust-1.41). It affects probably crates which were uploaded recently.

Trying to change disabled="" to disabled in Webdeveloper does not seem to have any effect. The disabled Attribute is probably not supported at all by Waterfox; the browser is based on Firefox 56 and disabled requires version 68 at least.

@jyn514
Copy link
Member

jyn514 commented Aug 1, 2020

https://rust-lang.github.io/rfcs/1985-tiered-browser-support.html#supported-browsers

Goal: Ensure functionality of our web content for 80% of users.
Browsers:

  • last 1 Firefox version
  • Firefox ESR

The latest ESR of firefox is 68: https://www.mozilla.org/en-US/firefox/68.0esr/releasenotes/

So I'm inclined to close this as wontfix.

@ensc
Copy link
Author

ensc commented Aug 1, 2020

Accordingly https://developer.mozilla.org/en-US/docs/Web/HTML/Element/link#Browser_compatibility it is not supported by Safari either (which is in your list).

As already noted, page above says about this attribute "Deprecated. Not for use in new websites." and "Non-standard. Expect poor cross-browser support".

@Nemo157
Copy link
Member

Nemo157 commented Aug 1, 2020

Changing styles appears to work fine on recent mobile and desktop Safari, so if it's not supported, it's not supported in a way which doesn't break anything.

@jyn514 jyn514 closed this as completed Aug 1, 2020
@ensc
Copy link
Author

ensc commented Aug 1, 2020

Nevertheless, it is bad to rely on deprecated and proprietary extensions and break websites completely.

@jyn514
Copy link
Member

jyn514 commented Aug 1, 2020

@ensc I understand your viewpoint and I realize this is frustrating. However the rustdoc team has limited time and resources, and we have to choose how to spend it. If you have ideas how to fix this feel free to make a PR and I (or another team member) would be happy to review it.

@Cldfire
Copy link
Contributor

Cldfire commented Aug 2, 2020

@ensc I apologize for the problem I created here with my PR! It's never fun to have one's setup broken by changes made elsewhere, and for that I am sorry.

I was actually unaware that the disabled attribute on link elements is discouraged from use, so thank you for bringing that to our attention here, I appreciate it.

As others in this thread have made clear, the browser support target around here is fairly recent. This is a consequence of the fact that most of us are unpaid volunteers, and while we do care deeply about the end users of Rust, we are limited in the time and energy we posses to keep things working for older and / or more niche setups, unfortunately.

That being said, I definitely agree that it would be in everyone's best interest to move away from the use of the disabled attribute on link elements here. I will take a look tomorrow and see if it's feasible to replace it with something better.

In particular, it appears that the mdbook project uses StyleSheet.disabled (as seen here), and that method is not deprecated and appears to have much better browser support.

Using that instead could perhaps improve the situation here for all parties concerned.

@Cldfire
Copy link
Contributor

Cldfire commented Aug 2, 2020

Opened #75063 which will fix this problem for you @ensc 😄

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.

5 participants