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 focus related race conditions #3700

Merged

Conversation

absidue
Copy link
Member

@absidue absidue commented Jun 25, 2023

Fix focus related race conditions

Pull Request Type

  • Bugfix

Related issue

closes #3444

Description

Fixes errors caused by focus being called on elements that Vue hasn't created yet. setTimeout() with no delay, just makes code run asynchronously, regardless of whether Vue has had time to create the elements or not. When you hold the R key on the trending tab, you trigger the refresh before Vue has had time to modify any HTML, so the element that we want to focus is never created. You might think that using Vue's nextTick() function would be more appropriate, however as that delays until after the next DOM update, it would mean that in this case the code would be triggered multiple times (however many times the refresh finished while you were holding the R key), when you finally let go of the R.

While I was at it, I decided to make the same change in other places that we call focus with a delay, which should hopefully fix similar errors caused by spamming/abusing FreeTube in those areas (looking at you @efb4f5ff-1298-471a-8973-3d47447115dc with your auto clicker 🙄).

Testing

On the trending page, hold the R key and check that no errors appear in the console.

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) June 25, 2023 16:15
@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Jun 25, 2023
@PikachuEXE
Copy link
Collaborator

I found it strange to keep refreshing when I hold down R key
Isn't this another bug? (not saying it should be fixed in this PR)

@absidue
Copy link
Member Author

absidue commented Jun 26, 2023

When you are holding it down, it only listens to the R key again after it has finished loading (you can add console.logs to confirm that). We can add some artificial timeout after the refresh has finished if you want that?

@PikachuEXE
Copy link
Collaborator

I only know now keydown event is also fired repeatedly when button is being hold

I guess FT is not using the repeat property?
https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/repeat

Reloading continuously on key holding is an unexpected behaviour for me, not sure what other people think

@FreeTubeBot FreeTubeBot merged commit 87a389c into FreeTubeApp:development Jun 27, 2023
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Jun 27, 2023
@absidue absidue deleted the fix-focus-race-conditions branch June 27, 2023 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Trending Uncaught TypeError: Cannot read properties of undefined (reading 'focus')
5 participants