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

Fix going back in history to a search result page on firefox #72272

Merged

Conversation

GuillaumeGomez
Copy link
Member

This bug was actually firefox not re-running JS script when you go back in history. To trigger it on the current docs:

  • Make a search
  • Pick an element (which isn't on the same page as the current element!)
  • Go back in history

Instead of having the search results, you'll see the normal doc page. You can find a small explanation about it here.

r? @kinnison

cc @ollie27

@kinnison
Copy link
Contributor

I am utterly unable to say if this makes any kind of sense. JS shennanigans at this level is beyond my understanding. I'm prepared to trust you when you say it works, but I'm not in a position to build and test locally right now.

@ollie27
Copy link
Member

ollie27 commented May 16, 2020

I haven't been able to reproduce this issue locally. Is this a known Firefox bug? Are we even using window.onload?

@GuillaumeGomez
Copy link
Member Author

The problem isn't on the JS side but on the browser side and seems to be specific to firefox. Like I said: the script isn't re-run unless we have the unload function set for the reason linked in the PR description. And since it's not re-run, the search gets discarded.

I recommend you to test locally with a little http server (and firefox!) to experience it: python3 -m http.server in the doc folder. :)

@GuillaumeGomez
Copy link
Member Author

Here's a video from my computer on doc.rust-lang.org/nightly/std:

doc-bug

When I come back to the previous page (using alt+left), you can see search results briefly before they disappear super quickly.

@kinnison
Copy link
Contributor

So I can confirm that I see this happen iff I use keyboard to navigate to a search result (i.e. down arrow and enter). If I use a mouse then it doesn't happen. This suggests there might be some kind of continuation of execution after the enter key press maybe, once the page is restarted with alt+left?


// This is required in firefox. Explanations: when going back in the history, firefox doesn't re-run
// the JS, therefore preventing rustdoc from setting a few things required to be able to reload the
// previous search results (if you clicked on a search result and came back).
Copy link
Contributor

Choose a reason for hiding this comment

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

After a long discussion on Discord, I'm okay providing:

Suggested change
// previous search results (if you clicked on a search result and came back).
// previous search results (if you navigated to a search result with the keyboard,
// pressed enter on it to navigate to that result, and then came back to this page).

@kinnison
Copy link
Contributor

With a comment rework akin to my suggestion, r=me

@GuillaumeGomez GuillaumeGomez force-pushed the fix-back-on-page-with-search-behaviour branch from f867818 to ed84780 Compare May 19, 2020 14:48
@GuillaumeGomez
Copy link
Member Author

It was a long but nice discussion. I learned a lot of things. :) I updated the comment, waiting for the CI and then let's go! Thanks for your reviews!

@GuillaumeGomez
Copy link
Member Author

@bors: r=kinnison

@bors
Copy link
Contributor

bors commented May 19, 2020

📌 Commit ed84780 has been approved by kinnison

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label May 19, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 20, 2020
…h-search-behaviour, r=kinnison

Fix going back in history to a search result page on firefox

This bug was actually firefox not re-running JS script when you go back in history. To trigger it on the current docs:

 * Make a search
 * Pick an element (which isn't on the same page as the current element!)
 * Go back in history

Instead of having the search results, you'll see the normal doc page. You can find a small explanation about it [here](http://web.archive.org/web/20100428053932/http://www.firefoxanswer.com/firefox/672-firefoxanswer.html).

r? @kinnison

cc @ollie27
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 20, 2020
…h-search-behaviour, r=kinnison

Fix going back in history to a search result page on firefox

This bug was actually firefox not re-running JS script when you go back in history. To trigger it on the current docs:

 * Make a search
 * Pick an element (which isn't on the same page as the current element!)
 * Go back in history

Instead of having the search results, you'll see the normal doc page. You can find a small explanation about it [here](http://web.archive.org/web/20100428053932/http://www.firefoxanswer.com/firefox/672-firefoxanswer.html).

r? @kinnison

cc @ollie27
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 20, 2020
…h-search-behaviour, r=kinnison

Fix going back in history to a search result page on firefox

This bug was actually firefox not re-running JS script when you go back in history. To trigger it on the current docs:

 * Make a search
 * Pick an element (which isn't on the same page as the current element!)
 * Go back in history

Instead of having the search results, you'll see the normal doc page. You can find a small explanation about it [here](http://web.archive.org/web/20100428053932/http://www.firefoxanswer.com/firefox/672-firefoxanswer.html).

r? @kinnison

cc @ollie27
bors added a commit to rust-lang-ci/rust that referenced this pull request May 21, 2020
Rollup of 7 pull requests

Successful merges:

 - rust-lang#71854 (Make `std::char` functions and constants associated to `char`.)
 - rust-lang#72111 (rustc-book: Document `-Z strip=val` option)
 - rust-lang#72272 (Fix going back in history to a search result page on firefox)
 - rust-lang#72296 (Suggest installing VS Build Tools in more situations)
 - rust-lang#72365 (Remove unused `StableHashingContext::node_to_hir_id` method)
 - rust-lang#72371 (FIX - Char documentation for unexperienced users)
 - rust-lang#72397 (llvm: Expose tiny code model to users)

Failed merges:

r? @ghost
@bors bors merged commit e279bd5 into rust-lang:master May 21, 2020
@GuillaumeGomez GuillaumeGomez deleted the fix-back-on-page-with-search-behaviour branch May 22, 2020 11:23
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 27, 2021
Fix back-forward cache in rustdoc frontend

Rustdoc's frontend set a no-op unload handler, specifically to disable
Firefox's back-forward cache because it caused a bug. It's nice to
allow the back-forward cache because it permits faster navigations.

This change addresses the issues that were caused by back-forward cache.

https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/1.5/Using_Firefox_1.5_caching
https://web.dev/bfcache/

Demo: https://jacob.hoffman-andrews.com/rust/fix-bfcache/std/string/struct.String.html

Related: rust-lang#72272
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants