-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Fix anchor invalid redirection to search #41950
Conversation
src/librustdoc/html/static/main.js
Outdated
var search = document.getElementById('search'); | ||
if (hasClass(main, 'content')) { | ||
addClass(main, 'hidden'); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be something like the following to avoid shadowing the search
function and to add the class to the correct element:
var search_c = document.getElementById('search');
if (hasClass(search_c, 'content')) {
addClass(search_c, 'hidden');
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent point!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was the issue actually. I thought the variable was limited to the scope, big mistake!
src/librustdoc/html/static/main.js
Outdated
// perform the search. This will empty the bar if there's | ||
// nothing there, which lets you really go back to a | ||
// previous state with nothing in the bar. | ||
document.getElementsByClassName('search-input')[0].value = params.search; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If params.search
is undefined
this should set the value to ""
not "undefined"
like it currently does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
src/librustdoc/html/static/main.js
Outdated
var search = document.getElementById('search'); | ||
if (hasClass(main, 'content')) { | ||
addClass(main, 'hidden'); | ||
if (location.href.indexOf(".html?search=") !== -1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would break clearing the search results so should be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems it's still working. So not sure if I tested what you had in mind.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh my bad, I need to test on a non local path.
ff6dd78
to
b09a19b
Compare
Updated. |
@bors r+ rollup |
📌 Commit b09a19b has been approved by |
…wsxcv Fix anchor invalid redirection to search Fixes rust-lang#41933. r? @rust-lang/docs
…wsxcv Fix anchor invalid redirection to search Fixes rust-lang#41933. r? @rust-lang/docs
Fixes #41933.
r? @rust-lang/docs