Make WordPress Core

Opened 16 years ago

Closed 13 years ago

Last modified 4 months ago

#5683 closed defect (bug) (worksforme)

New tag section needs display filtering

Reported by: jhodgdon's profile jhodgdon Owned by: ryan's profile ryan
Milestone: Priority: normal
Severity: normal Version: 2.5
Component: Taxonomy Keywords: has-patch has-unit-tests
Focuses: Cc:


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.

Change History (24)

#1 @jhodgdon
16 years ago

  • Keywords 2nd-opinion added

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?

#2 @ryan
16 years ago

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

#3 @jhodgdon
16 years ago

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.

#4 @jhodgdon
16 years ago

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.

#5 @jhodgdon
16 years ago

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

#6 @jhodgdon
16 years ago

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.


#7 @ryan
16 years ago

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.

#8 @jhodgdon
16 years ago

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.

#9 @ryan
16 years ago

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

#10 @jhodgdon
16 years ago

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?

#11 @Denis-de-Bernardy
15 years ago

  • Component changed from General to Taxonomy
  • Owner changed from anonymous to ryan

is this still current? the scripts got updated recently.

#12 @jhodgdon
15 years ago

Yes, this is all still current.

#13 @Denis-de-Bernardy
15 years ago

  • Keywords needs-patch added; tags 2nd-opinion removed

#14 @ryan
15 years ago

  • Milestone changed from 2.9 to Future Release

#15 @kevinB
14 years ago

  • Cc kevinB added

#16 @solarissmoke
13 years ago

  • Keywords needs-patch removed
  • Resolution set to worksforme
  • Status changed from new to closed

This is fixed in trunk

#17 @dd32
13 years ago

  • Milestone Future Release deleted

This ticket was mentioned in PR #5721 on WordPress/wordpress-develop by @dmsnell.

6 months ago

  • Keywords has-patch added

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:

@zieladam commented on PR #5721:

6 months ago

@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


@dmsnell commented on PR #5721:

6 months ago

@adamziel everything should be buttoned up and working properly now

This ticket was mentioned in PR #5975 on WordPress/wordpress-develop by @jonsurrell.

4 months ago

  • Keywords has-unit-tests added

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]

@jonsurrell commented on PR #5975:

4 months ago

@dmsnell your fix looks good to me and fixes the test failures I saw in

👍 This change looks good to me.

Note: See TracTickets for help on using tickets.