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 no longer honours --default-theme #87263

Closed
ijackson opened this issue Jul 18, 2021 · 8 comments · Fixed by #87288
Closed

rustdoc no longer honours --default-theme #87263

ijackson opened this issue Jul 18, 2021 · 8 comments · Fixed by #87288
Assignees
Labels
A-rustdoc-js Area: Rustdoc's JS front-end C-bug Category: This is a bug. P-medium Medium priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@ijackson
Copy link
Contributor

Steps to reproduce

git clone https://salsa.debian.org/iwj/rust-vecdeque-stableix # any crate will do
cd rust-vecdeque-stableix
RUSTDOCFLAGS='--default-theme ayu' cargo doc
dir=$PWD
rm -rf d
mkdir d
cd d
HOME=$PWD firefox file://$dir/target/doc/vecdeque_stableix/index.html

Expected results

The documentation appears in the "ayu" (dark) theme.

Observed results

The documentation appears in the default (light) theme.

Notes

This works on stable. I haven't bisected it or anything.

Empirically, changing the default theme once works for any one browser profile with the default firefox settings. AIUI the setting is intended to be recorded in web local storage. However, my firefox configuration does not permit use of web storage or cookies, so it does not work. This is one of the reasons for the --default-theme option's existence.

FTR I see this message in my JS console which seems to support this hypothesis: Request to access cookie or storage on “file:///home/rustcargo/Rustup/Game/vecdeque-stableix/target/doc/vecdeque_stableix/struct.IntoIter.html” was blocked because we are blocking all storage access requests.

Meta

rustdoc 1.55.0-nightly (2f391da2e 2021-07-14)
binary: rustdoc
commit-hash: 2f391da2e6ac73faa3570b79de239fd8c0edf1a9
commit-date: 2021-07-14
host: x86_64-unknown-linux-gnu
release: 1.55.0-nightly
LLVM version: 12.0.1
@ijackson ijackson added the C-bug Category: This is a bug. label Jul 18, 2021
@ijackson
Copy link
Contributor Author

@rustbot modify labels +A-rustdoc-js +regression-from-stable-to-nightly

@rustbot rustbot added A-rustdoc-js Area: Rustdoc's JS front-end regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jul 18, 2021
@ijackson
Copy link
Contributor Author

FTR, a faster test is available if you hae a browser which blocks cookies and web storage: then you can just reload the page, as it always shows whatever the docs' built-in default is.

@jyn514
Copy link
Member

jyn514 commented Jul 18, 2021

@ijackson does this also happen on beta?

@jyn514 jyn514 added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Jul 18, 2021
@ijackson
Copy link
Contributor Author

Beta is fine.

@ijackson
Copy link
Contributor Author

ijackson commented Jul 19, 2021

The first bad nightly is 406d4a9cc 2021-06-21; the last good nightly is e82b65026 2021-06-20 (in each case from rustup ... <the following date>.

git-log suggests #86157 may be implicated. The diffs do seem to suggest that it would be easy to make a mistake there. Weirdly, https://github.com/rust-lang/rust/pulls/86157 is 404 even though bors says in 9d93819 "Auto merge of #86157" oops botched the url. That is the right MR ref.

@ijackson
Copy link
Contributor Author

(FTR, @jyn514, I am 100% in favour of using Tera instead of all those write! calls.)

@ijackson
Copy link
Contributor Author

I have found the problem: the generated HTML now says:

<script id="default-settings" 
        data-theme="ayu"
        data-use-system-theme="false"></script>

instead of

<script id="default-settings" data-theme="ayu" data-use_system_theme="false"></script>

The formatting is of no consequence, but the changes of _ to - are causing the breakage. (I eyeballed this yesterday but didn't spot this difference, seeing only what I expected...)

I suspect that this may be the result of Tera's autoescaping function. Things are subtle here (primarily because of the crazy way the names of data- attributes are translated into the DOM) and the Tera defaults are probably not appropriate.

I will look at the code and send an MR.

@apiraino
Copy link
Contributor

Assigning priority as discussed in the Zulip thread of the Prioritization Working Group.

@rustbot label -I-prioritize +P-medium

@rustbot rustbot added P-medium Medium priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jul 22, 2021
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jul 22, 2021
…Gomez

rustdoc: Restore --default-theme, etc, by restoring varname escaping

In rust-lang#86157

    cd0f931
    Use Tera templates for rustdoc.

dropped the following transformation from the keys of the default settings element's `data-` attribute names:

    .map(|(k, v)| format!(r#" data-{}="{}""#, k.replace('-', "_"), Escape(v)))

The `Escape` part is indeed no longer needed, because Tera does that for us.  But the massaging of `-` to `_` is needed, for the (bizarre) reasons explained in the new comments.

I have tested that the default theme function works again for me.  I have also verified that passing (in shell syntax)

    '--default-theme="zork&"'

escapes the value in the HTML.

Closes rust-lang#87263
@bors bors closed this as completed in 155b055 Jul 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-js Area: Rustdoc's JS front-end C-bug Category: This is a bug. P-medium Medium priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. 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