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

Going back from source requires three clicks #764

Closed
mahkoh opened this issue May 20, 2020 · 8 comments · Fixed by #873
Closed

Going back from source requires three clicks #764

mahkoh opened this issue May 20, 2020 · 8 comments · Fixed by #873
Labels
A-frontend Area: Web frontend E-easy Effort: Should be easy to implement and would make a good first PR P-medium Medium priority

Comments

@mahkoh
Copy link

mahkoh commented May 20, 2020

See https://a.pomf.cat/qyfvmb.webm

@jyn514
Copy link
Member

jyn514 commented May 20, 2020

I can't replicate with firefox, what browser are you using? https://a.pomf.cat/ohdprb.mkv

@mahkoh
Copy link
Author

mahkoh commented May 20, 2020

I'm using chromium. I believe the cause is this part of the site which changes the location object:

    var doc_body = document.getElementById("rustdoc_body_wrapper");
    if(window.location.hash) {
        var notFirefox = typeof InstallTrigger === 'undefined';
        if(notFirefox) {
            var hash = window.location.hash;
            window.location.hash = "";          <-------------
            setTimeout(function () {
                window.location.hash = hash;         <-------------
                doc_body.focus();
            }, 1);
        }
    } else {
        doc_body.focus();
    }

According to the spec, under some arcane conditions, changing the hash causes a new history entry to be created (in the spec language: the previous entry to not be replaced): https://html.spec.whatwg.org/multipage/history.html#location-object-setter-navigate

@jyn514
Copy link
Member

jyn514 commented May 20, 2020

cc @GuillaumeGomez , @Nemo157 , this looks related to #750

@jyn514
Copy link
Member

jyn514 commented May 20, 2020

Would a possible workaround be to remove the messing around with window.location.hash and instead just call doc_body.focus() in the setTimeout?

@GuillaumeGomez
Copy link
Member

I'm not even sure why it has been done like this in the first place...

@Nemo157
Copy link
Member

Nemo157 commented May 20, 2020

I couldn't figure out what the focus calls were even meant to be doing, or how to navigate the menu with the keyboard, which is why I was very glad to find out just a css change in #750 was enough to fix the performance and I could just ignore the JS.

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented May 20, 2020

Then maybe let's remove this code and see if anyone complains? And if someone complains, add a comment this time. XD

@Kixiron Kixiron added the A-frontend Area: Web frontend label May 27, 2020
@jyn514 jyn514 added E-easy Effort: Should be easy to implement and would make a good first PR P-medium Medium priority labels Jul 1, 2020
@jyn514
Copy link
Member

jyn514 commented Jul 1, 2020

Relevant code:

var doc_body = document.getElementById("rustdoc_body_wrapper");

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-frontend Area: Web frontend E-easy Effort: Should be easy to implement and would make a good first PR P-medium Medium priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants