New tag section needs display filtering

[jhodgdon]

As of 2.5-bleeding [6627], the new tag entry section on the admin screen displays un-filtered tag names. This is an issue for users of some multi-lingual plugins (and perhaps others), for whom raw tag names from the database are unsightly and unwieldy. It would be much better, and consistent, if tags were run through the standard "display" filters before putting them on the screen.

This applies to the type-ahead drop-down list that displays tags that match what you are typing, and also to the area below that lists tags you've already entered with delete buttons next to them.

I'll see about a patch, hopefully in the next day or two.

[jhodgdon]

I took a look at how this was coded up. A simple patch isn't possible, in my opinion. The code would need to be quite a bit more complex to handle the following:

a) Raw tag names may not be appropriate to display to the user. They should be filtered with "display" filtering.

b) Tag names filtered with "display" filtering, if put into the hidden tag input field, will not match up in the database when the post is saved, so the code will end up creating new tags instead of matching previously-entered tags. Slugs will match, or un-filtered raw tag names.

c) If someone puts a "display" filtered tag name into the dynamic text entry field and clicks "add", you would want to match with existing tags in the database -- same problem as (b) if you don't.

Right now, the code is assuming that the same text can be displayed to the user, matched when someone types a tag and clicks "add", and put into the hidden tag input field to match the DB when the post is saved. Which works fine for the majority of users, who are operating in a single language, don't use display filtering on their tags, and don't need to edit their tag slugs so they are readable in URLs.

To fix it for those users in more complex situations, so that it will display filtered text, match on "add", and put slugs or unfiltered raw names into the hidden tag entry field makes things quite a bit more complex, because the filtering is done in PHP, matching is done in MySQL, and the display is done in JS.

My approach in the Advanced Tag Entry plugin (which does handle display vs. slug vs. raw) was to do away with free text entry and just use drop-down lists for everything -- that works, because you can do the filtering at load time and save it in a JS array. But it doesn't work well for users that have thousands of tags -- it takes forever to load the tag info into JS.

Probably an approach that includes a free text field or avoids loading all the tags initially into JS can't easily be made to work with filtering... so I should just continue to maintain something like the Advanced Tag Entry plugin for my multilingual plugin users.

And probably this bug should be closed as "won't fix". Thoughts?

[ryan]

tag saving should move to using IDs the way categories do. Passing the names back and forth causes several problems.

[jhodgdon]

Yeah, that would be better. The problem is that if people have 1000 tags (which I've heard some people do, or at least 100s of them), you don't probably want to display all 1000 of them in drop-down lists, checkboxes, links, etc.

[jhodgdon]

All that would be needed would be to convert the Tags section to look like Categories currently does.

Maybe with the addition of a little "search" feature, which would narrow down the list of displayed tags, like "starts with..." or something like that.

[jhodgdon]

And then if you could add the slug to the Add Category or Add Tag section, that would be even better... :)

[jhodgdon]

Ryan (or whoever): Any ideas what you want to do about this? I probably have some time today to do some programming.

The current (as of [6757]) situation for mutilingual users on the post editing screen is this:

• Category names in the Category block are displayed with display filtering, and there's a clear distinction between using existing categories and creating a new one (good)
• Tag names displayed in the lower section of the Tags block are not display filtered (ugly, inconsistent with Category section behavior and other screens, probably usable for all but novice multilingual users)
• Type-ahead tag names are not display filtered, but at least the type-ahead search finds substrings rather than restricting to the beginning of the tag name (ugly, inconsistent, probably usable)
• No clear distinction in Tags block UI between adding a brand-new tag to the DB and attaching an existing tag to a post (inconsistent with Category block)
• Add New Category and Add Tag do not allow you to provide a slug (essential for multilingual users)

What I would like to see:

1. Put a slug field on the Add New Category form (easy)
2. Make a distinction on Tag section between adding existing tags and creating new tags, and have a slug entry field on Create New Tag form (easy)
3. Ideally, display-filter everything in the Tags section. However, as mentioned in previous comments, this would require moving to a checkbox or drop-down system similar to the Categories section -- complete rewrite -- and might not be workable for bloggers with tons of tags (difficult)

I could create patches for (a) and (b) today (I think), but I don't want to bother if the patches have little chance of being applied. My thought is that (a) and (b) would be useful to more than just multilingual users, and that multilingual users could live without (c) if necessary. They can live without (a) and (b) as well, but will have to add all tags and categories from the management screens, rather than doing them on the fly while editing posts.

Thoughts?

[ryan]

Slug fields are purposefully left off, and I think they would be unneeded added complexity for most people. Adding a slug field for tags would be especially tricky since there can be multiple comma-separated tags.

I'm not sure how to approach this. i18n plugins could add new category and tag boxes that have the needed UI (perhaps using #5798) and we could make sure the server-side AJAX can handle passed slugs. Could be clunky though.

[jhodgdon]

As another point in favor of slugs on the "create new" sections, I believe that single-language users of WP who write in Chinese, Japanese, and other non-url-friendly languages also create slugs for their categories and tags, since otherwise the URLs are a string of %02%83 etc and totally unreadable. So I do think that slugs are useful to a fairly wide audience. It's not a huge deal, but having slugs on the Create New sections would allow these users to get the full benefit of those sections of the post edit screen.

I would also only suggest adding a slug field to a new, separate "create a new tag" expandable area, and the existing "Add New Category" area, not to the existing "Add a tag" field. Agreed: it would be way too tricky to deal with multiple tags and multiple slugs.

Anyway, if you don't want slugs in those sections, then we can just tell all multilingual and non-European-language bloggers to use the Manage Tag/Category screens whenever they create new tags or categories.

[ryan]

Okay, I'll send this to the UI folks and see what they come up with.

[jhodgdon]

Any update on this idea of adding slugs to the Add Tag and Add Category sections of the post editing screen? Would you like me to file a separate bug, as I think my original bug is not going to be fixed?

[Denis-de-Bernardy]

is this still current? the scripts got updated recently.

[jhodgdon]

Yes, this is all still current.

[solarissmoke]

This is fixed in trunk

This patch follows-up with earlier design questions around how to represent spans of strings inside the class. It's relevant now as preparation for #5683.

The mixture of (offset, length) and (start, end) coordinates becomes confusing at times and all final string operations are performed with the (offset, length) pair, since these feed into strlen().

In preparation for exposing all tokens within an HTML document this change:

• Unifies the representation throughout the class.
• It creates token_starts_at to track the start of the current token.
• It replaces tag_ends_at with token_length for re-use with other token types.

There should be no functional or behavioral changes in this patch.

For the internal helper classes this patch introduces breaking changes, but those classes are marked private and should not be used outside of the HTML API itself.

Trac ticket:

@dmsnell The unit tests fail now. Once they're fixed and the @since annotations are adjusted, we should be ready to merge.

1) Tests_HtmlApi_WpHtmlProcessorBreadcrumbs::test_remains_stable_when_editing_attributes
Undefined variable $bookmark_start

/var/www/src/wp-includes/html-api/class-wp-html-tag-processor.php:1654
/var/www/src/wp-includes/html-api/class-wp-html-tag-processor.php:2331
/var/www/tests/phpunit/tests/html-api/wpHtmlProcessorBreadcrumbs.php:449
/var/www/vendor/bin/phpunit:122

@adamziel everything should be buttoned up and working properly now

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

Trac ticket: Core-60382

When next_token() was introduced in #5683, it introduced regression in the HTML Processor whereby void tags remain on the stack of open elements when they shouldn't. This led to invalid values returned from get_breadcrumbs().

The reason was that calling next_token() works through a different code path than the HTML Processor runs everything else. To solve this, its sub-classed next_token() called step( self::REPROCESS_CURRENT_TOKEN ) so that the proper HTML accounting takes place.

Unfortunately that same _reprocessing_ code path skipped the step whereby void and self-closing elements are popped from the stack of open elements.

In this patch that step is run through the introduction of a third mode for step(), which is self::PROCESS_CURRENT_TOKEN. This mode acts as if self::PROCESS_NEXT_NODE were called, except it doesn't advance the parser, that already having been done by next_token() first.

Follow-up to [57348]

@dmsnell your fix looks good to me and fixes the test failures I saw in ​https://github.com/WordPress/wordpress-develop/pull/5794.

👍 This change looks good to me.

Merged in [57507]
​https://github.com/wordpress/wordpress-develop/commit/cdb218b2007693262f2ea2d9717ec0ff55620b80