Can't create categories with different names but similar slugs on Edit Post screen

[jeffreymcmanus]

We run a blog that deals with programming languages. Today we were creating a post concerning the C programming language and attempted to create a new category for it, called "C" (just one character). This angered WordPress; it appeared to create a superfluous "Uncategorized" category, but it wound up not actually creating any categories for the post.

I've attached a screenie so you can see what this looks like after I attempt to create the category.

If it's for some reason forbidden to create categories comprised of a single character, maybe a validation error message of some kind would be in order?

[jeffreymcmanus]

Screen shot of error

[linuxologos]

I can reproduce that, if I try to create a category with a name resembling another category but only through the Add New Category on the Add New Post screen. Creating the category through Posts > Categories succeeds. So the problem is not the single-character category name in general.

Try to create the category through Posts > Categories to see if it works.

[linuxologos]

The bug is simple to reproduce. Just try to create a category that already exists or something like C, C#, C+, C! etc. (they have to be parent categories). "Uncategorized" is constantly appearing, but nothing is really created (try to click one of those "Uncategorized" and all of them are chosen) and refreshing the page makes them disappear.

This applies to WP 3.1-RC4 too.

[jeffreymcmanus]

Confirmed that it is possible to create a single-letter category through Posts > Categories (which I never knew existed until today!).

[garyc40]

Can't reproduce on trunk (currently on r17453).

[garyc40]

Wait, I can reproduce now. It happens when there's already a "C#" category, and you try to add "C".

[garyc40]

The conditional check on line 263, admin-ajax.php seems to be unnecessary, and introduces edge case like this.

Removing the conditional check seems to fix this problem and produce no side effects (the same kind of check is performed within wp_insert_term, and it handles the case better by creating the term anyways with a numerical suffix).

Attached patch does this 16567.diff​.

[garyc40]

In case someone wonders why "Uncategorized" is output, it's because the return value of term_exists() can also be an array if a taxonomy is specified (wp-includes/taxonomy.php:1438). wp_terms_checklist() later casts this value as an int, making it 1 because it's an array. As a result, what you get is whatever category has the ID 1 (in this case, Uncategorized).

By the way, the inline documentation of term_exists() states that:

@return mixed Get the term id or Term Object, if exists.

Judging from the code I see, the return value could only be either an associative array, or an integer, but never a term object. Or did I miss something?

[MikeHansenMe]

I cant seem to product this in trunk (3.5-alpha-21751), even with the example C# then C from the edit posts page.

[SergeyBiryukov]

The code is still there, and I can reproduce the issue in both trunk and 3.4.2.

1. Go to Edit Post screen.
2. Create "C#" category.
3. Try to create "C" category. It won't be created, "C#" will be checked instead.

The only difference from 3.0.5 is that "Uncategorized" item no longer gets added to the list in step 3. That was fixed in [19792] (for #17939).

A check for empty name was added in [6303]. is_term() was added in [12798] as a check for an existing term, and replaced with term_exists() in [15220].

Looking at the code, it doesn't make much sense to check if a category with the same slug exists here. This is better handled further down the stack in wp_insert_term(), as noted by garyc40 in comment:6.

A check for empty name:
​http://core.trac.wordpress.org/browser/tags/3.4.2/wp-includes/taxonomy.php#L2031
A check for an existing term:
​http://core.trac.wordpress.org/browser/tags/3.4.2/wp-includes/taxonomy.php#L2063
trim() is a part of in sanitize_text_field():
​http://core.trac.wordpress.org/browser/tags/3.4.2/wp-includes/formatting.php#L3164

16567.2.diff​ removes unnecessary checks from ajax-actions.php.
is_array() is removed as well, since unlike term_exists(), wp_insert_term() always returns either an array or a WP_Error object.

[SergeyBiryukov]

This isn't limited to single-character categories.

[technosailor]

The problem is query_var collision. sanitize_title() is going to strip special chars like # in C# for the slug. So C# does in fact become 'c' as does C++ or just C.

I don't agree that wp_insert_term should be where the checks are happening. It adds a potentially unneeded query. We should not be doing wp_insert_term if term_exists. I believe the existing functionality is as it should be. I respect the problem but it's really edge and a core behavior modification just doesn't feel warranted. That's my opinion though.

[SergeyBiryukov]

Replying to technosailor:

I don't agree that wp_insert_term should be where the checks are happening. It adds a potentially unneeded query. We should not be doing wp_insert_term if term_exists.

Quite the contrary, 16567.2.diff​ prevents term_exists() from being called twice if the term does not exist, since wp_insert_term() calls it anyway.

[technosailor]

Replying to SergeyBiryukov:

Replying to technosailor:

I don't agree that wp_insert_term should be where the checks are happening. It adds a potentially unneeded query. We should not be doing wp_insert_term if term_exists.

Quite the contrary, 16567.2.diff​ prevents term_exists() from being called twice if the term does not exist, since wp_insert_term() calls it anyway.

Then wp_insert_term() shouldn't be calling it. We shouldn't be doing an insert if there is a collision as that adds a DB call.

[ericlewis]

Replying to technosailor:

Replying to SergeyBiryukov:

Replying to technosailor:

I don't agree that wp_insert_term should be where the checks are happening. It adds a potentially unneeded query. We should not be doing wp_insert_term if term_exists.

Quite the contrary, 16567.2.diff​ prevents term_exists() from being called twice if the term does not exist, since wp_insert_term() calls it anyway.

Then wp_insert_term() shouldn't be calling it. We shouldn't be doing an insert if there is a collision as that adds a DB call.

Changing the functionality of wp_insert_term seems out of the scope of this ticket.

16567.2.diff works for me.

[SergeyBiryukov]

Seems like #17689 would fix this ticket as well.

However, the sanitization in _wp_ajax_add_hierarchical_term() and wp_ajax_add_link_category() still seems redundant (see comment:9), so 16567.2.diff​ might still make sense.

[SergeyBiryukov]

#24841 was marked as a duplicate.

[boonebgorges]

I believe this ticket was resolved with [34809]. See #33864.

[pavelevap]

It is still broken in WordPress 4.7. I can create both "C" and "C+" categories in wp-admin/edit-tags.php?taxonomy=category, but second category is not created on Edit Post screen (there is no error message).

[boonebgorges]

@pavelevap Yes, you're correct. 16567.4.diff​ looks like the right fix to me. term_exists() should be banished to the flames.

[boonebgorges]

In 39637:

Taxonomy: Eliminate redundant and inaccurate dupe check when creating categories from post.php.

The term_exists() check is not needed because of existing dupe
checks in wp_insert_term(). Furthermore, term_exists() conflates
term names and sanitized slugs, so incorrectly marks terms like
'C' and 'C+' as duplicates of one another.

Props garyc40, SergeyBiryukov, kovshenin, MikeHansenMe.
Fixes #16567.