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

rustdoc: Fix events not being registered when navigating browser history #95674

Closed

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Apr 5, 2022

It might be linked to #93784.

I realized lately that in some cases, I needed to press ENTER for the rustdoc search to be performed on firefox. After inspecting the DOM events, I saw that the rustdoc events were simply not registered, meaning the registerSearchEvents needed to be run (again?). I suspect it's the same issue that forces us to call the search function for firefox when we navigate through history.

I created new functions where we use addEventListener to prevent to register a same event more than once by first removing it by using removeEventListener.

You can test it online here.

r? @jsha

@GuillaumeGomez GuillaumeGomez added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-rustdoc-js Area: Rustdoc's front-end labels Apr 5, 2022
@rust-highfive
Copy link
Collaborator

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez,@Folyd,@jsha

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 5, 2022
@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez
Copy link
Member Author

I uploaded an online version as well here.

@jsha
Copy link
Contributor

jsha commented Apr 6, 2022

in some cases, I needed to press ENTER for the rustdoc search to be performed on firefox.

Are you able to provide more detail about those cases? Do they happen when you've navigated backwards or forwards to a search page? I tried to reproduce on Firefox and could not.

After inspecting the DOM events, I saw that the rustdoc events were simply not registered, meaning the registerSearchEvents needed to be run (again?).

This is surprising. If none of the search events were registered, how would hitting Enter trigger a search?

I suspect it's the same issue that forces us to call the search function for firefox when we navigate through history.

I doubt it. When navigating forward and back, we use the back-forward cache (bfcache). That cache preserves page JS state. That cache also preserves form state like search fields. However, our search field has autocomplete=off, which has these semantics:

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.

So we re-run the search function in order to copy the query from the URL into the search box (we can likely be much more efficient here, by just copying the query and not re-searching).

So the bfcache issue shouldn't cause us to be missing event handlers after a back navigation.

@GuillaumeGomez
Copy link
Member Author

GuillaumeGomez commented Apr 7, 2022

I can reproduce it every time with:

  • Go to page
  • Make a search
  • Click on a result search
  • Go back to previous page (search results)
  • Press 's' and enter something new
  • Nothing happens

I discovered it the first time after waking up my computer.

@jsha
Copy link
Contributor

jsha commented Apr 7, 2022

Weird; I can't reproduce on Firefox 99.0, Ubuntu. On either stable or nightly on doc.rust-lang.org.

Does this reproduce for you in a fresh profile? What version of Firefox?

@GuillaumeGomez
Copy link
Member Author

Firefox 99 on ubuntu. Default profile (so "stable" I assume).

@jsha
Copy link
Contributor

jsha commented Apr 7, 2022

By "stable" I meant https://doc.rust-lang.org/std (as opposed to https://doc.rust-lang.org/nightly/std). Does it reproduce on both for you?

By "fresh profile" I meant: if you run firefox --no-remote --profile "$(mktemp -d)" does this reproduce for you?

@GuillaumeGomez
Copy link
Member Author

I have the problem on stable, beta and nightly. I'll need to check with the firefox profile you provided.

@GuillaumeGomez
Copy link
Member Author

When running with firefox --no-remote --profile "$(mktemp -d)", it says:

Your firefox profile cannot be loaded. It may be missing or inaccessible.

Did you try after a computer sleep wake up?

@bors
Copy link
Contributor

bors commented Apr 26, 2022

☔ The latest upstream changes (presumably #96428) made this pull request unmergeable. Please resolve the merge conflicts.

@jsha
Copy link
Contributor

jsha commented May 29, 2022

Your firefox profile cannot be loaded. It may be missing or inaccessible.

That's odd. Try this instead:

mkdir /tmp/clean-profile
firefox --no-remote --profile /tmp/clean-profile

Did you try after a computer sleep wake up?

Yes.

@GuillaumeGomez
Copy link
Member Author

Didn't have time to come back to this but I found a weird case to "fix" this: resizing the viewport.

@jsha jsha added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 20, 2022
@JohnCSimon
Copy link
Member

@GuillaumeGomez
ping from triage - can you post your status on this PR? This has sat unmoved for a while now. Thanks!

FYI: when a PR is ready for review, send a message containing
@rustbot ready to switch to S-waiting-on-review so the PR is in the reviewer's backlog.

@GuillaumeGomez
Copy link
Member Author

GuillaumeGomez commented Mar 21, 2023

I got some more information about this. Here's a scenario which allows me to repeat this bug every time (on firefox, linux):

  • Go to any rustdoc web page (either local or remote, doesn't matter).
  • Enter sleep mode with computer.
  • Leave sleep mode.
  • Enter a search... Nothing happens

What's interesting is that when I look at the events registered on the <input>, I only see "focus" and none of the others. I'll rebase this PR.

@GuillaumeGomez GuillaumeGomez deleted the events-not-registered branch July 12, 2023 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-js Area: Rustdoc's front-end S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants