Skip to content

Fix JS search scripts path #145650

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix JS search scripts path #145650

wants to merge 1 commit into from
+2 −2

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Aug 20, 2025 •
edited by rustbot
Loading

rootPath always end with a / so we should not add one. Interestingly enough, it only triggers the bug on a website (like https://doc.rust-lang.org/nightly/std/).

Follow-up of #144476.
Fixes #145646.

cc @notriddle
r? @fmease

@rustbot
Copy link
Collaborator

rustbot commented Aug 20, 2025

fmease is not on the review rotation at the moment.
They may take a while to respond.

@rustbot rustbot added A-rustdoc-search Area: Rustdoc's search feature S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. labels Aug 20, 2025
@rustbot
Copy link
Collaborator

rustbot commented Aug 20, 2025

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @jsha, @lolbinarycat

@rust-log-analyzer

This comment has been minimized.

Sign in to view
lqd
lqd reviewed Aug 20, 2025
View reviewed changes
@GuillaumeGomez GuillaumeGomez force-pushed the fix-js-search-scripts-path branch from d15d582 to e1045c2 Compare August 20, 2025 10:57
@fmease
Copy link
Member

fmease commented Aug 20, 2025

Interestingly enough, it only triggers the bug on a website

Locally, if we're dealing with file://, the URL also contains double slashes (e.g, file:///home/fmease/programming/rust/rustc3/doc//search.index/path/d30b089bc95f.js) but at least on Linux consecutive slashes get normalized to a single one, so it ends up resolving.

fmease
fmease approved these changes Aug 20, 2025
View reviewed changes
Copy link
Member

@fmease fmease left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

I've manually gone through all references of ROOT_PATH and "DocSearch#rootPath" (set to ROOT_PATH in the ctor) and all other sites seem to append path segments correctly (i.e., without an extra slash).

@fmease
Copy link
Member

fmease commented Aug 20, 2025

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Aug 20, 2025

📌 Commit e1045c2 has been approved by fmease

It is now in the queue for this repository.

@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 Aug 20, 2025
Zalathar added a commit to Zalathar/rust that referenced this pull request Aug 20, 2025
@Zalathar
Rollup merge of rust-lang#145650 - GuillaumeGomez:fix-js-search-scrip…
141a1e8 
…ts-path, r=fmease

Fix JS search scripts path

`rootPath` always end with a `/` so we should not add one. Interestingly enough, it only triggers the bug on a website (like https://doc.rust-lang.org/nightly/std/).

Follow-up of rust-lang#144476.
Fixes rust-lang#145646.

cc `@notriddle`
r? `@fmease`
@Zalathar Zalathar mentioned this pull request Aug 20, 2025
Closed
@Zalathar
Copy link
Contributor

I've taken the liberty of rolling this up (despite the backlog) because I think it's worthwhile to get doc search working again ASAP.

bors added a commit that referenced this pull request Aug 20, 2025
@bors
Auto merge of #145658 - Zalathar:rollup-ccqb8hk, r=Zalathar
339e4d8 
Rollup of 5 pull requests

Successful merges:

 - #144915 (Defer tail call ret ty equality to check_tail_calls)
 - #145256 (Add new `--test-codegen-backend` bootstrap option)
 - #145415 (std_detect: RISC-V: implement implication to "C")
 - #145647 (miri subtree update)
 - #145650 (Fix JS search scripts path)

r? `@ghost`
`@rustbot` modify labels: rollup
@lolbinarycat
Copy link
Contributor

Locally, if we're dealing with file://, the URL also contains double slashes (e.g, file:///home/fmease/programming/rust/rustc3/doc//search.index/path/d30b089bc95f.js) but at least on Linux consecutive slashes get normalized to a single one, so it ends up resolving.

most http servers also deduplicate consecutive slashes, but apparently not the one that rust documentation is hosted on.

@blyxyas
Copy link
Member

blyxyas commented Aug 20, 2025 •
edited
Loading

It also affected docs.rs, so Axum doesn't deduplicate slashes it seems.
tokio-rs/axum#714 might be related?

@fmease
Copy link
Member

fmease commented Aug 20, 2025

That just means that rustdoc can't assume that consecutive slashes get normalized away by whomever ends up interpreting the path.

@lolbinarycat
Copy link
Contributor

That just means that rustdoc can't assume that consecutive slashes get normalized away by whomever ends up interpreting the path.

Is there a way we can modify the gui testing framework to error if there are ever duplicated slashes in the URL path? that seems desirable to stop this from happening again.

@fmease
Copy link
Member

fmease commented Aug 20, 2025 •
edited
Loading

CC https://en.wikipedia.org/wiki/URI_normalization which classifies "slash collapsing" to not necessarily be semantics-preserving.

@notriddle
Copy link
Contributor

Yes. I'll have a PR open that tests this in a few minutes.

@notriddle notriddle mentioned this pull request Aug 20, 2025
Open
jhpratt added a commit to jhpratt/rust that referenced this pull request Aug 20, 2025
@jhpratt
Rollup merge of rust-lang#145650 - GuillaumeGomez:fix-js-search-scrip…
be5ee77 
…ts-path, r=fmease

Fix JS search scripts path

`rootPath` always end with a `/` so we should not add one. Interestingly enough, it only triggers the bug on a website (like https://doc.rust-lang.org/nightly/std/).

Follow-up of rust-lang#144476.
Fixes rust-lang#145646.

cc `@notriddle`
r? `@fmease`
@jhpratt jhpratt mentioned this pull request Aug 20, 2025
Closed
bors added a commit that referenced this pull request Aug 20, 2025
@bors
Auto merge of #145680 - jhpratt:rollup-cokwvpc, r=jhpratt
5bea2c6 
Rollup of 12 pull requests

Successful merges:

 - #143383 (stabilize `const_array_each_ref`)
 - #144443 (Make target pointer width in target json an integer)
 - #144758 ([Doc] Add links to the various collections)
 - #144915 (Defer tail call ret ty equality to check_tail_calls)
 - #145256 (Add new `--test-codegen-backend` bootstrap option)
 - #145415 (std_detect: RISC-V: implement implication to "C")
 - #145573 (Add an experimental unsafe(force_target_feature) attribute.)
 - #145642 (Do not use effective_visibilities query for Adt types of a local trait while proving a where-clause)
 - #145650 (Fix JS search scripts path)
 - #145654 (Download CI GCC into the correct directory)
 - #145662 (Enforce correct number of arguments for `"x86-interrupt"` functions)
 - #145674 (Enable triagebot `[review-changes-since]` feature)

Failed merges:

 - #145647 (miri subtree update)

r? `@ghost`
`@rustbot` modify labels: rollup
jhpratt added a commit to jhpratt/rust that referenced this pull request Aug 20, 2025
@jhpratt
Rollup merge of rust-lang#145650 - GuillaumeGomez:fix-js-search-scrip…
fcd44a7 
…ts-path, r=fmease

Fix JS search scripts path

`rootPath` always end with a `/` so we should not add one. Interestingly enough, it only triggers the bug on a website (like https://doc.rust-lang.org/nightly/std/).

Follow-up of rust-lang#144476.
Fixes rust-lang#145646.

cc ``@notriddle``
r? ``@fmease``
@jhpratt jhpratt mentioned this pull request Aug 20, 2025
Closed
bors added a commit that referenced this pull request Aug 20, 2025
@bors
Auto merge of #145681 - jhpratt:rollup-7hn0ii7, r=jhpratt
5a72bc9 
Rollup of 14 pull requests

Successful merges:

 - #143383 (stabilize `const_array_each_ref`)
 - #144443 (Make target pointer width in target json an integer)
 - #144758 ([Doc] Add links to the various collections)
 - #144915 (Defer tail call ret ty equality to check_tail_calls)
 - #145137 (Consolidate panicking functions in `slice/index.rs`)
 - #145256 (Add new `--test-codegen-backend` bootstrap option)
 - #145297 (fix(debuginfo): handle false positives in overflow check)
 - #145415 (std_detect: RISC-V: implement implication to "C")
 - #145642 (Do not use effective_visibilities query for Adt types of a local trait while proving a where-clause)
 - #145650 (Fix JS search scripts path)
 - #145654 (Download CI GCC into the correct directory)
 - #145662 (Enforce correct number of arguments for `"x86-interrupt"` functions)
 - #145674 (Enable triagebot `[review-changes-since]` feature)
 - #145678 (Fix typo in docstring)

r? `@ghost`
`@rustbot` modify labels: rollup
jhpratt added a commit to jhpratt/rust that referenced this pull request Aug 21, 2025
@jhpratt
Rollup merge of rust-lang#145650 - GuillaumeGomez:fix-js-search-scrip…
a885918 
…ts-path, r=fmease

Fix JS search scripts path

`rootPath` always end with a `/` so we should not add one. Interestingly enough, it only triggers the bug on a website (like https://doc.rust-lang.org/nightly/std/).

Follow-up of rust-lang#144476.
Fixes rust-lang#145646.

cc ```@notriddle```
r? ```@fmease```
@jhpratt jhpratt mentioned this pull request Aug 21, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lqd lqd lqd left review comments

@fmease fmease fmease approved these changes

Assignees

@fmease fmease

Labels
A-rustdoc-search Area: Rustdoc's search feature 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. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Search loads indefinitely in doc.rust-lang.org/nightly due to 404
10 participants
@GuillaumeGomez @rustbot @rust-log-analyzer @fmease @bors @Zalathar @lolbinarycat @blyxyas @notriddle @lqd