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

Multiple autocompleters can crash #41709

Closed
johngodley opened this issue Jun 14, 2022 · 4 comments
Closed

Multiple autocompleters can crash #41709

johngodley opened this issue Jun 14, 2022 · 4 comments
Assignees
Labels
[Type] Bug An existing feature does not function as intended

Comments

@johngodley
Copy link
Contributor

johngodley commented Jun 14, 2022

Description

If you have multiple autocompleters then they can cause a crash or browser freeze.

I've traced this down to #41382, specifically this line:

https://github.com/WordPress/gutenberg/pull/41382/files#diff-9cfb2f5fedb0b684384a354cc972272fc659f9cb6e66ea5dcd77bb15feacd989R382

If you have more than one autocompleter then it seems to cause an infinite loop. If you're lucky this results in a crash, if you're not the browser freezes.

Step-by-step reproduction instructions

It's a little hard to recreate on a default Gutenberg, but you can do the following:

  1. Edit packages/editor/src/hooks/default-autocompleters.js and add a copy of the user autocompleter but with a different trigger prefix. Compile this.
function setDefaultCompleters( completers = [] ) {
	// Provide copies so filters may directly modify them.
	completers.push( clone( userAutocompleter ) );
	completers.push( clone( { ...userAutocompleter, triggerPrefix: '+' } ) );

	return completers;
}
  1. Start a new post and type a combination of the autocompleters (no need to wait for the autocompleter):
    +user @user +user @user +user
  2. At some point your browser will freeze. It is stuck in an infinite loop in the useEffect. I suspect that the useEffect is causing the completers dependency to change and so it keeps being called.

Screenshots, screen recording, code snippet

No response

Environment info

  • Gutenberg 13.4.0
  • WordPress 6.0
  • Any browser

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@johngodley johngodley added the [Type] Bug An existing feature does not function as intended label Jun 14, 2022
@johngodley
Copy link
Contributor Author

Looping @chad1008 in for thoughts!

@chad1008
Copy link
Contributor

Good catch, thanks for the ping @johngodley. I can definitely reproduce... I'll look into a fix!

@chad1008 chad1008 self-assigned this Jun 15, 2022
@chad1008
Copy link
Contributor

Update: Current testing points to filteredOptions.length being the new dependency that's causing the issue.

I have a tentative fix working locally when combined with the fixes in a separate, but related PR (#41749). I want to dig a little deeper into the details of what was triggering the loop, and how, to make sure this fix isn't missing anything else.

@chad1008
Copy link
Contributor

The PR introducing this issue has been reverted (##41820) and will land in the v13.5 release next week, so I'm closing this for now. I'll be taking another pass at that refactor in the future and will keep this regression in mind to avoid it going forward!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

2 participants