Skip to content

Fix anchor invalid redirection to search - #41950

Merged
bors merged 1 commit into
rust-lang:masterfrom
GuillaumeGomez:rustdoc-links
May 14, 2017
Merged

Fix anchor invalid redirection to search#41950
bors merged 1 commit into
rust-lang:masterfrom
GuillaumeGomez:rustdoc-links

Conversation

@GuillaumeGomez

@GuillaumeGomezGuillaumeGomez commented May 12, 2017

Copy link
Copy Markdown
Member

Fixes#41933.

r? @rust-lang/docs

ollie27
ollie27 suggested changes May 12, 2017
View reviewed changes
Comment threadsrc/librustdoc/html/static/main.js Outdated

@ollie27ollie27May 12, 2017

Copy link
Copy Markdown
Contributor

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:

varsearch_c=document.getElementById('search');if(hasClass(search_c,'content')){addClass(search_c,'hidden');}

@GuillaumeGomezGuillaumeGomezMay 12, 2017

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Excellent point!

@GuillaumeGomezGuillaumeGomezMay 12, 2017

Copy link
Copy Markdown
MemberAuthor

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!

Comment threadsrc/librustdoc/html/static/main.js Outdated

@ollie27ollie27May 12, 2017

Copy link
Copy Markdown
Contributor

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.

@GuillaumeGomezGuillaumeGomezMay 12, 2017

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

👍

Comment threadsrc/librustdoc/html/static/main.js Outdated

@ollie27ollie27May 12, 2017

Copy link
Copy Markdown
Contributor

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.

@GuillaumeGomezGuillaumeGomezMay 12, 2017

Copy link
Copy Markdown
MemberAuthor

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.

@GuillaumeGomezGuillaumeGomezMay 12, 2017

Copy link
Copy Markdown
MemberAuthor

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.

@shepmastershepmaster added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 12, 2017
@GuillaumeGomez

GuillaumeGomez commented May 12, 2017

Copy link
Copy Markdown
MemberAuthor

Updated.

@GuillaumeGomezGuillaumeGomez added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-waiting-on-team and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 13, 2017
@frewsxcv

frewsxcv commented May 13, 2017

Copy link
Copy Markdown
Contributor

@bors r+ rollup

@bors

bors commented May 13, 2017

Copy link
Copy Markdown
Collaborator

📌 Commit b09a19b has been approved by frewsxcv

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request May 13, 2017
@Mark-Simulacrum
Rollup merge ofrust-lang#41950- GuillaumeGomez:rustdoc-links, r=fre…
e7c4628
…wsxcv
Fix anchor invalid redirection to search
Fixesrust-lang#41933.
r? @rust-lang/docs
@Mark-SimulacrumMark-Simulacrum mentioned this pull request May 13, 2017
Closed
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request May 14, 2017
@Mark-Simulacrum
Rollup merge ofrust-lang#41950- GuillaumeGomez:rustdoc-links, r=fre…
24aba55
…wsxcv
Fix anchor invalid redirection to search
Fixesrust-lang#41933.
r? @rust-lang/docs
@Mark-SimulacrumMark-Simulacrum mentioned this pull request May 14, 2017
Merged
bors added a commit that referenced this pull request May 14, 2017
@bors
Auto merge of#41987- Mark-Simulacrum:rollup, r=Mark-Simulacrum
94497b7
Rollup of 7 pull requests
- Successful merges: #41612, #41826, #41939, #41946, #41950, #41975, #41979
- Failed merges:
@bors
bors merged commit b09a19b into rust-lang:masterMay 14, 2017
@GuillaumeGomez
GuillaumeGomez deleted the rustdoc-links branch May 14, 2017 10:12
@apirainoapiraino removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-waiting-on-team labels May 25, 2023
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers
@frewsxcvfrewsxcvfrewsxcv approved these changes
@ollie27ollie27ollie27 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Clicking on rustdoc sidebar item results in 'undefined' page

6 participants

@GuillaumeGomez@frewsxcv@bors@ollie27@shepmaster@apiraino