Quick edit creates new tag from existing tag-name

[forposts]

Product version:
2.8 svn from trunk, revision 10843.

Steps to reproduce:
1) Make some post.
2) Create new tag with "tag1" slug / "tag1" name, assign in to the post from 1).
3) Create second tag with "tag2" slug and "tag2 name" name. Assign it to the same post.
4) Go to /wp-admin/edit.php then choose Quick Edit for the post in question.
You'll see two terms "tag1, tag2 name" in Tags window.
Don't touch anything and push Update Post button.
5) Go to /wp-admin/edit-tags.php?taxonomy=post_tag
you'll find new unwanted tag with "tag2-name" slug.

I consider this a bug because of wrong behaviour - system must assign "tag2" slug to the post instead of making new slug from "tag2 name".

It looks like tag order is critical, the system works fine if you change tag places and enter "tag2 name, tag1" in Quick Edit window. In this case undesirable tag is not created.

[forposts]

Same bug detected for WP 2.7.1

It seems that new "unauthorized" tag popping up when two conditions are met - it must be not the first tag in a row in Quick Edit AND tag slug and tag name are different.

Example.
Original tag list for the post, slug/name: "tag1"/"tag1 name", "tag2"/"tag2 name","tag2"/"tag3 name". If you click on Update Post in Quick Edit you get two new tags created, "tag2 name"/"tag2 name" and "tag3 name"/"tag3 name". That's bad behaviour for sure.

[hakre]

Quick Edit seems to be not working correctly. Is there a way to provide a set of unit-test to check if Quick Edit was ever working safe, strict and properly?

[Denis-de-Bernardy]

probably a dup of #6593

[jhodgdon]

I don't think it is a duplicate, because this bug happens in Quick Edit and #6593 is in the full Edit screen. Also, #6593 is only happening in 2.8-bleeding for tags with apostrophes in them, whereas this bug does not involve apostrophes.

However, they may have related causes.

[Denis-de-Bernardy]

it's the same js being used in the background. but you might be right (which is why I didn't close this :-) ).

[jhodgdon]

I don't think it's the JS that is causing tag duplication in #6593. At least last time I looked at it (around when I first created #6593, so several versions ago) the problem was in the PHP code that is taking the form POST submission, and from that figuring out which tags the user meant to associate with the post (i.e. corresponding the POST submission to term IDs).

That is why I think if the POST had the term IDs in it rather than trying to do text matching, we would avoid all these problems. That's what the categories section does, and it doesn't suffer from this problem.

I haven't ever looked into what Quick Edit is doing, though, so I have no idea if the problem is in JS or PHP for this bug. Likely it is in the PHP -- when you submit the quick edit, the info is submitted via AJAX (I think) and it is definitely PHP code that saves the post.

Then again, maybe you meant to say "same PHP being used in the background" rather than JS. :) Not sure where you are, but it's definitely the end of a long day here...

[jhodgdon]

Just as one more note, one of the reproduces from #6593 that is currently working in 2.8-bleeding was the tag "San Andreas", which has a space in it. So although I didn't test "tag2 name" in the full edit screen, I don't think spaces are an issue there, as they apparently are in Quick Edit.

[Denis-de-Bernardy]

Replying to jhodgdon:

Then again, maybe you meant to say "same PHP being used in the background" rather than JS. :) Not sure where you are, but it's definitely the end of a long day here...

A bit of both, actually. The stuff got revamped recently -- I had to update a few plugins of mine. And yeah, it *is* getting late over here in France. :D

[hakre]

I've just could reproduce it with the current 2.8 Trunk. From the view from above, Tag Parsing of Quickedit is not properly working here. I'll take a look inside.

[hakre]

Quickedit uses (via 4 or 5 other functions) wp_set_object_terms() with an $append=false. Read yourself:

* @param array|int|string $term The slug or id of the term, will replace all existing 
* related terms in this taxonomy.

Actually passed is an array that contains the names, not slugs, not ids.

First all ids for the post are pulled. ids are numbers, not the names nor slugs.

Then the passed names (not slugs) as terms are passed one by one to is_term() which tries to find out by slug or name. that function mainly is for forming two sql queries who should look up for a particular tag by name or slug.

that function then calls sanitize_title() which replaces spaces. therefore the ones named tag has been converted to slug. but that is only slug by accident because the correct slug of 'tag2 name' is 'tag2' not 'tag2-name'.

because a tag with the slug 'tag2-name' can not be found, it will be inserted with the appropriate name ('tag2 name'). this is why a new tag is added. that tag will be discovered (new id) and the one with the old id won't be discovered any longer and therefore removed ($append = false).

how to patch?

I have no Idea. This is all so mixed up, something sanitzed here, something re-interpreted there. I doubt touching any of these functions is really promising.

i therefore set this to wontfix. feel free to provide a solution/patch and reopen then.

[jhodgdon]

So you set it to "won't fix" just because you cannot think of a solution? How about setting to "needs patch"?

This is actually a pretty serious bug, because many users do custom slug tags (for instance, non-European languages and multi-lingual blogs). Setting it to "won't fix" means that no one will give it a second look to see if they can find a solution. Bad idea, in my opinion.

[hakre]

Sorry to have you bugged with that. I explained why I set that to wontfix and it has taken me about one and a half hour to only understand why that actually is happening. I even know a way of how _that_ bug can be fixed but I assume then major other parts of the Backend won't work any longer. That is why I set that to wontfix, because to fix this, there is a lot more work to do.

As written, feel free to provide actually a concept how to fix or - even better - a patch.

I raised the Priority and Severity for you.

[hakre]

To get a view what all is happening, why and it's history, grab yourself a cup of coffee and read #6593. That helps to get an insight.

the problem with the current code is, that it is only a fragment around the database. there is no real concept (compare #6593), it is a fixing here and a fixing there and if it seems to work, the patch is accepted.

can someone provide a written concept of how taxnonomies should work?

[jhodgdon]

OK. This may be a slight simplification, but here it is, including some history.

Tags and categories are both "taxonomies", and a taxonomy is a set of terms.

In the database, there's a table that associates term IDs, taxonomy IDs, and post IDs. That is what tells WordPress which category and tag IDs (all numbers) are associated with each post.

There is another table that associates term IDs with term names and term slugs. This is what tells WordPress how to display a given term and figure out its URL.

When you edit a post (via full or quick edit), WP has to figure out which terms (categories or tags) the user meant to associate with the post, and save the IDs in the taxonomy/term/post association table.

Categories are easy (and in my opinion done the right way). When you edit a post (on the main edit screen), category choices are displayed with their displayed term names, but the edit screen behind the scenes is working with term IDs. So when you save the post, the desired term IDs are sent in the $_POST data, so there's no question as to which categories go with the post.

Tags are not so easy. When tags were first introduced in WP, the way they worked is that when you edited a post, there was a plain text field where you could type in your desired tags, separated by commas. When you saved the post, WP received the typed-in term names in $_POST. Then WP would sanitize the term names to convert them to valid URL slugs (there was no way to customize slugs for tags then except with a plugin). Then WP would look up the generated slugs in the term table to find the term IDs, add missing tags to the term table to get new IDs, and then associate all those term IDs with the post.

In more current versions of WP, you can customize your term slugs, and this lookup scheme broke down. It's been somewhat patched (see the long history of #6593), but obviously it isn't totally working. And just as a note, although on the main post edit screen you don't see a plain text field with tags separated by commas, behind the scenes that is exactly what is there (the field is simply hidden if you have JavaScript enabled).

What I think needs to be done to fix this bug is to overhaul the way tags are selected for a post on both the Quick Edit screen and the full Edit screen, so that they work more like categories. That is, whatever the user interface is, when you click on, type, or otherwise select a tag, behind the scenes of the UI it should only be working with tag IDs. To make things compatible with how things have been in WP, you'd still want a way to type in new tags, probably, since they're supposed to be kind of free-form (that's the whole tag philosophy, isn't it?).

Does that help?

[jhodgdon]

I'm going to do my best at a patch for this and #6593 tomorrow.

[Denis-de-Bernardy]

in case you're unfamiliar with it, you'll want to use the SCRIPT_DEBUG define in a wp-config.php file, to make use of the post.dev.js file rather than the mostly unreadable post.js file.

[jhodgdon]

Thanks! I am sure that will be helpful, and I wasn't aware of it.

[hakre]

I think there is another route that is doable: Do not lookup Tags by their Slugs but by their actual names. That will prevent the Bug of this ticket as well. The Bug only existst because it is done only for looking for the (guessed) slug.

The Bug would be prevented if the lookup would be done by only looking for the name not a (guessed) slug.

[jhodgdon]

The main Post Edit screen does look up by name. But even that still has a problem if the name has an apostrophe in it, so there is still some problem.

[jhodgdon]

Fix for this problem

[jhodgdon]

All that worry. It turns out the reason this bug was happening was that somewhere in WP it assumed there were no spaces in the tags field, but the tags field had spaces.

So when the quick edit went to be saved, WP was exploding on commas, and the 2nd and subsequent terms started with spaces.

When WP then tried to find the tags in the DB to see if they already existed, the name with a leading space didn't match the name in the DB. Ergo, must be a new term.

The patch I just added simply trims the term name before trying to find it in the DB. I don't think this is a bad idea ever. Terms shouldn't start or end with spaces, and I think that when WP creates new terms, the spaces get stripped anyway. Not sure about that? Anyway, this fixes the problem. No more duplicate tags on quick edit.

Also, just as a note, the Quick Edit stuff that saves the tags is going through the same code as full Post Edit. So the fix to #6593 that I posted a few minutes back also fixes apostrophe issues here that probably existed.

[hakre]

Passed that. Because of not finding with Name, it creates new based on slug. Okay, I can follow.

Tested your patch, it works''' No new tags created.

[ryan]

Trim $term before creating the slug so we don't end up with trailing or leading dashes?

[hakre]

the term (not slug!) must not be created because it already exists and already was attached to the post object. the input must be trimmed to get the correct name sothat it can be looked up in the terms table as being already linked with the object.

[ryan]

Agreed. But if we trim term we should do it before creating the slug.

[jhodgdon]

Ah. I didn't test that -- assumed the sanitize function was smart enough to trim, but perhaps it isn't?

I'll fix that patch.

[jhodgdon]

Better patch - trims before creating slugs (see comments below)

[jhodgdon]

How about this patch? Works for me (fixes the issue reported here), and trims before the slug gets created.

[ryan]

(In [11151]) Trim term before checking if it already exists. Props jhodgdon. fixes #9397

[hakre]

Second patch works as well for me. Test: OK, no additional category is created after submitting the testdata via the quickedit form.