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 back-forward cache in rustdoc frontend #82511

Merged
merged 1 commit into from
Feb 27, 2021
Merged

Conversation

jsha
Copy link
Contributor

@jsha jsha commented Feb 25, 2021

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: #72272

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/
@rust-highfive
Copy link
Collaborator

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ollie27 (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 25, 2021
@jsha
Copy link
Contributor Author

jsha commented Feb 25, 2021

There's another, somewhat simpler fix that eliminates the need for any new JS: We can just remove the autocomplete="off" property on the search-input. I've tested this and it works beautifully.

According to https://developer.mozilla.org/en-US/docs/Web/Security/Securing_your_site/Turning_off_form_autocompletion:

Setting autocomplete="off" on fields has two effects:

  • It tells the browser not to save data inputted by the user for later autocompletion on similar forms, though heuristics for complying vary by browser.
  • It stops the browser from caching form data in the session history. When form data is cached in session history, the information filled in by the user is shown in the case where the user has submitted the form and clicked the Back button to go back to the original form page.

That second bullet point explains why we had this bad interaction with the bfcache. Normally, the input field would be preserved across forward/back navigations. But because it has autocomplete="off", it was empty on returning to the page. Rustdoc's JS saw the empty input and treats that as a signal to clear the search.

@GuillaumeGomez
Copy link
Member

To be noted that the search function is now called twice by default on any page (except that it does nothing since there is no search query). It's really minor and I think that the improvement itself is definitely better!

Thanks a lot @jsha !

@bors: r+

@bors
Copy link
Contributor

bors commented Feb 26, 2021

📌 Commit 51a14c7 has been approved by GuillaumeGomez

@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 Feb 26, 2021
@bors
Copy link
Contributor

bors commented Feb 27, 2021

⌛ Testing commit 51a14c7 with merge 0846043...

@bors
Copy link
Contributor

bors commented Feb 27, 2021

☀️ Test successful - checks-actions
Approved by: GuillaumeGomez
Pushing 0846043 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 27, 2021
@bors bors merged commit 0846043 into rust-lang:master Feb 27, 2021
@rustbot rustbot added this to the 1.52.0 milestone Feb 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. 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.

6 participants