The tag field hangs the browsers when you have a bunch of tags

[pikamander2]

One of the recent core updates made the tags feature nearly unusable when you have a bunch of tags. We have tens of thousands of tags and it used to work fine, but now it freezes after typing the first few letters.

Other people are reporting the same issue here: ​https://wordpress.org/support/topic/classic-editor-page-freezes-when-tags-is-used/

Steps to reproduce:

1. Create tens of thousands of tags
2. Start editing a post
3. Try typing in the name of a tag
4. The browser will hang and ask if you want to abort the tab

It sounds to me like there are two underlying issues here:

1. The JavaScript is not asynchronous
2. The JavaScript immediately makes the network request rather than waiting a second or two after a letter is typed.

[pikamander2]

It looks like Gutenberg limits it to 20:

/wp-json/wp/v2/tags?context=view&per_page=20&orderby=count&order=desc&_fields=id%2Cname&search=a%20t&_locale=user

While the Classic Editor has no limit:

/wp-admin/admin-ajax.php?action=ajax-tag-search&tax=post_tag&q=the&_fs_blog_admin=true

Would it be possible to limit the Classic Editor to 20 as well, either by default or at least with a filter?

[pikamander2]

When searching for tags via AJAX in the Classic Editor, limit the results to 20 to match Gutenberg and prevent browser hangs/crashes on sites that have thousands of tags

[pikamander2]

@SergeyBiryukov - I've attached a patch that:

1. Adds support for specifying the number of terms to be returned (the default remains unlimited like before for the sake of backwards compatibility) in the wp_ajax_ajax_tag_search function in ajax-actions.php

2. Sets the relevant GET variable to 20 in the Classic Editor's AJAX call in tags-suggest.js to match the Gutenberg behavior and prevent browser crashes on sites that have thousands of tags

[costdev]

@pikamander2 Thanks for the patch!

A couple of comments:

1. ​intval() returns int, so is_int() isn't needed here.
2. WP_Term_Query::parse_query() performs absint( $query['number'] ), so intval() isn't needed here.

Therefore, the changes to wp_ajax_ajax_tag_search() could just be adding this line to the array:

'number'     => isset( $_GET['number'] ) ? $_GET['number'] : 0,

@SergeyBiryukov I think adding test coverage for this would ultimately just be covering get_terms(), but let me know if you want some unit tests for this change.

[pikamander2]

@costdev @SergeyBiryukov - Thanks for the review. I've implemented the changes in the patch below.

[pikamander2]

When searching for tags via AJAX in the Classic Editor, limit the results to 20 to match Gutenberg and prevent browser hangs/crashes on sites that have thousands of tags

[azouamauriac]

Hello @pikamander2 thanks for the patch! I'm not really sure if limit the result number is a good idea, since some result will match but they won't be show up because of limit.

[pikamander2]

There needs to be some kind of default limit so that the feature doesn't break sites with a large amount of tags. I chose 20 results because it matches the Gutenberg behavior. If the user's desired term doesn't show up, then they can continue typing more characters just like they would in Gutenberg.

That said, we could also apply a filter to make the number customizable. I'll attach a third patch below with support for that functionality.

[pikamander2]

When searching for tags via AJAX in the Classic Editor, limit the default number of results to 20 (filterable) to match Gutenberg and prevent browser hangs/crashes on sites that have thousands of tags.

[pikamander2]

Usage example (limit to 5 results):

add_filter('term_search_num_results', function($number){return 5;});

Usage example (limit to 50 results):

add_filter('term_search_num_results', function($number){return 50;});

Usage example (no limit):

add_filter('term_search_num_results', function($number){return null;});

[costdev]

@pikamander2 I see the latest patch has introduced a new filter, term_search_num_results.

Some thoughts:

• The limit could be included in a minor release as a maintenance fix given the reason for the ticket, but I'm not sure if a filter should be introduced in a minor release or if 20 should added as a limit in 5.9.1 and the filter added in 6.0. One consideration that may make this suitable for a minor release is that 20 may not be the preferred limit for some. That's for the Release Lead / Committer to decide though.
• The filter should have a docblock. See the other filters in the file for examples.
• The filter may benefit from being applied on a line above the array given the line length now. (i.e. $number = apply_filters(...)
• Something to consider later: Since this filter allows for a difference between the Gutenberg limit and Classic limit, should the filter also be included where Gutenberg sets the limit to 20, if possible?

[pikamander2]

A maintenance fix sounds like a good start since it's actively hurting owners of large sites. We could raise the default to something higher like 100 if that's really necessary, although I would think that the best approach would be to match the Gutenberg behavior for now (tag-limit-fix.2.patch) and then introduce a proper filter for both functions in the next minor release.

[costdev]

I agree and consider the maintenance fix a necessity for 5.9.1.

I also agree that matching Gutenberg's behaviour makes sense. The filter, whether released in 5.9.1 or 6.0, will allow it to be set to a preferred value.

@azouamauriac I'm not really sure if limit the result number is a good idea, since some result will match but they won't be show up because of limit.

IMO, there is already a technical limit in the form of the feature freezing when too many tags exist. This change implements a safety limit to prevent that ever occurring.

There's at least two ways to do this:

1. Investigate all factors that cause the freeze, from the number of tags, to the amount of data (long tag names, etc).
2. Set a limit that's consistent with other behaviour in Core.

IMO, setting the limit to 20 makes sense for the following reasons:

1. Consistency in Core.
2. The amount of time it takes a user to enter one or two more characters to reveal their desired tag is significantly less than scrolling through a long list of possible matches.

[azouamauriac]

Hi @costdev @pikamander2 i've read your comments, here is an example of what i'm saying, let assume that we have two tags "a baby" and "baby", and we set the limit to one(1), when you search "baby" in tag box suggestion you will see only "a baby"(that's what happen on my side you can try and confirm), so how can i do to show the tag "baby" in search result?

Now if we expand the limit to 1 to 20 or any other number the same thing will happen when the number of tags that contain the search word will be more than the limit.

Although I'm not feeling comfortable with the limit solution I think it's currently the good solution we have for 5.9.1. But what I think should be best are these:

• first, ensure that the previous ajax request is done before launch new one.
• second, launch ajax request only when the previous result does not content the current search word, that assume that the previous result is saved somewhere and used as the source of search according to ​https://api.jqueryui.com/autocomplete/#option-source

thanks!

[audrasjb]

As per today's bug scrub:

• tag-limit-fix.2.patch looks almost good to go. We just need to remove the changes in the minified file :)
• We need to make sure it was introduced in 5.9 cycle. If not, let's move this ticket to 6.0

Trac ticket: https://core.trac.wordpress.org/ticket/55052

[audrasjb]

The above PR implements the mentioned patch. It refreshes the patch against trunk and adds int casting for the 'number' GET parameter.

[peterwilsoncc]

Replying to audrasjb:

We need to make sure it was introduced in 5.9 cycle. If not, let's move this ticket to 6.0

Was this determined? One of the comments in the support thread suggests this was happening in 5.8 and earlier too. If that's the case, it would be wise to put this in for release during the 6.0 cycle.

[hellofromTonya]

Looking at the ​first report of this issue in the Support forum, it was before 5.9 was released. I agree with @peterwilsoncc that this is a better fit for 6.0.

@audrasjb what do you think?

[audrasjb]

Yeah let's move this to 6.0 since it wasn't a regression introduced in 5.9.

I did consider whether the PHP default ought to be 20 too but realised it probably needs to stay at unlimited to avoid breaking themes or plugins using the endpoint and expecting all the results.

Yep, that was my line of thought as well. The PHP function feels like it should have a sane limit, but for backwards compatibility reasons it's probably best just to do it by parameter in the AJAX call.

[peterwilsoncc]

Testing notes:

1) Install classic-editor
2) Create around one thousand tags: wp term generate post_tag --count=1111 (Tag 1, Tag 2, ... Tag n)
3) Go to the classic editor interface
4) Search for the term tag in the tag search

Before

• impractically large search results dropdown
• I wasn't able to make it crash (even after adding another 5111 tags) but the AJAX request was slow

After

• Tags limited to a usable amount
• AJAX request faster
• number parameter confirmed to have an affect

[peterwilsoncc]

In 53089:

Editor: Limit display of tags on classic editor.

On the classic editor, limit the search of tags and non-hierarchical taxonomies to twenty results. This in turn prevents an unbounded database query via an AJAX request.

Props pikamander2, costdev, azouamauriac, audrasjb.
Fixes #55052.

Merged in https://core.trac.wordpress.org/changeset/53089