Unable to add Taxonomies to custom post type taxonomies in 3.4.1

[lightmensendme]

This is reproducible in at least IE8 and FF 13.0.1, WordPress 3.4.1, and at least two separate domains.

Problem: We have a least two sites which have custom post types and taxonomies originally added by the Custom Post Type UI plugin, in which we are unable to add new taxonomies to. After disabling all plugins and putting the post type/taxonomy code directly into the functions.php file: we are still unable to add new taxonomies.

Reproduction: 1. Add a custom post type and a custom post type taxonomy. Add new custom post type "entry/post" from the admin menu or from the Dashboard sidebar. If is a hierarchical custom post type taxonomy, add a new custom post taxonomy. When the "Add New [Taxonomy]" button is pressed, the entered name in the taxonomy text field stays present, the add button seems to disappear from its location, and the add button is now displayed at the end of the custom post taxonomy list. The new taxonomy has not been added. (see image attached)

2. Add a custom post type and a custom post type taxonomy. Add new custom post type "entry/post" from the admin menu or from the Dashboard sidebar. If is a hierarchical custom post type taxonomy, which already has taxonomy present, check several taxonomies. You will notice that there is "lag" in selecting these current taxonomies (IE8 at least).
3. Although I have not been able to reproduce this, a client has mentioned that on an add custom post page where there is custom post taxonomy, the page is unusable and can eventually crash in IE8.

Example functions.php

register_post_type('events', array(	'label' => 'Events','description' => '','public' => true,'show_ui' => true,'show_in_menu' => true,'capability_type' => 'post','hierarchical' => false,'rewrite' => array('slug' => ''),'query_var' => true,'supports' => array('title','editor','excerpt','trackbacks','custom-fields','comments','revisions','thumbnail','author','page-attributes',),'labels' => array (
  'name' => 'Events',
  'singular_name' => 'Event',
  'menu_name' => 'Events',
  'add_new' => 'Add Event',
  'add_new_item' => 'Add New Event',
  'edit' => 'Edit',
  'edit_item' => 'Edit Event',
  'new_item' => 'New Event',
  'view' => 'View Event',
  'view_item' => 'View Event',
  'search_items' => 'Search Events',
  'not_found' => 'No Events Found',
  'not_found_in_trash' => 'No Events Found in Trash',
  'parent' => 'Parent Event',
),) );

register_taxonomy('event-city',array (
  0 => 'events',
),array( 'hierarchical' => true, 'label' => 'Locations','show_ui' => true,'query_var' => true,'rewrite' => array('slug' => ''),'singular_label' => 'Location') );

[lightmensendme]

Screenshot after adding new custom post taxonomy

[SergeyBiryukov]

I can reproduce issues 1 and 2 in Firefox 13 and IE 8.

Introduced in [20337] with jQuery 1.7.2.

[markjaquith]

I reproduced issue 1 in Chrome 19 and Firefox 13.

[markjaquith]

"event-city" starts with the word "even". WordPress uses some colon-separated class names. like: add:event-citychecklist. See the :even in there? The issue manifests if you replace "event-city" with "oddsfish".

On line 81 of wp-includes/js/wp-lists.dev.js has the following line:

		if ( !e.is('[class^="add:' + list.id + ':"]') )

This test is failing when it shouldn't be.

Here's my theory: there was a bug introduced in jQuery 1.7.2 regarding attribute selector quoting. It's seeing :even or :odd in a quoted class selector and it's parsing that as the :even/:odd pseudo-classes that jQuery supports.

What's really frustrating is that I can't reproduce it more simply. I've been trying to make a simple proof of concept on jsfiddle.net

​http://jsfiddle.net/g4CCw/

Maybe someone else will have better luck.

[SergeyBiryukov]

21106.patch​ fixes issue 1 by checking id attribute instead of class.

[SergeyBiryukov]

We have similar code in at least one more place:
​http://core.trac.wordpress.org/browser/trunk/wp-includes/js/wp-lists.dev.js?rev=17981#L392

Not sure at a glance if it fails or not. Would be great to pinpoint the exact change in jQuery behaviour, but so far I could not reproduce it in a standalone example either.

[markjaquith]

Koopersmith says it's probably a really low level thing that changed in their selector engine. The main takeaway is that our use of colons within class names is really ill-advised.

[markjaquith]

#21152 for a general "kill colons in class names" ticket.

[markjaquith]

In [21205]:

Change a jQuery selector to work around a change in jQuery 1.7.2 related to some class names containing colons. props SergeyBiryukov. fixes #21106 for trunk. see #21152

[markjaquith]

Re-opening so we can consider it for a potential 3.4.2. Not sure this merits it all by itself, but if we have to release one anyway, might consider working it in.

[SergeyBiryukov]

Issue 2 (the lag in IE when selecting terms) still remains. Going to take a look by the weekend if no one beats me to it. Might be related to issue 3 as well.

[SergeyBiryukov]

Still needs a patch for issue 2.

[wonderboymusic]

I patched the colon classes ticket: https://core.trac.wordpress.org/ticket/21152 - perhaps that can be used in some form?

[markjaquith]

Patch to backport [21205] and [21688] to the 3.4 branch.

[SergeyBiryukov]

Replying to wonderboymusic:

I patched the colon classes ticket: #21152 - perhaps that can be used in some form?

Tested the patch in IE 7 and 8, doesn't fix the lagging when selecting a taxonomy term. Seems to be caused by something else.

[SergeyBiryukov]

Closed #21747 as a duplicate.

In trunk (with jQuery 1.8.1), the symptoms are different:

1. The lagging appears when checking any category on a regular Edit Post screen as well, in both IE and Firefox.
2. If there's a taxonomy starting with "even", IE throws an error when checking its terms:

   Stack overflow at line: 2
   

Upon debugging, I've found two particular places which seem to be slowing things down:

1. '[class^="add:' + list.id + ':"]:not(form)' selector in wpList.process(): ​http://core.trac.wordpress.org/browser/tags/3.4.1/wp-admin/js/post.dev.js#L345
   As far as I can see, it only runs once, but commenting it out resulted in a significant speedup. With the patch from #21152 the results were the same, which made me think the culprit here is :not(form) rather than the :even part. Removing :not(form) confirmed that.
2. '#' + taxonomy + 'checklist li.popular-category :checkbox selector in post.js: ​http://core.trac.wordpress.org/browser/tags/3.4.1/wp-includes/js/wp-lists.dev.js#L400 ​jQuery.com suggests replacing it with [type="checkbox"]:

   Because :checkbox is a jQuery extension and not part of the CSS specification, queries using :checkbox cannot take advantage of the performance boost provided by the native DOM querySelectorAll() method. For better performance in modern browsers, use [type="checkbox"] instead.
   

21106.2.patch​ fixes the stack overflow in IE and the lagging in both Firefox and IE. It should cover all the existing core usage of those class names. Tested in 3.4.1 (with jQuery 1.7.2) as well.

21106.3.patch​ binds all three non-form functions to both a and input elements, in case plugins might rely on that. Seems is a little bit slower, but perhaps safer. Not sure if that's necessary though.

[azaozz]

We should be able to escape the : when using the CSS classes in jQuery:

If you wish to use any of the meta-characters ( such as !"#$%&'()*+,./:;<=>?@[\]^`{|}~ )
as a literal part of a name, you must escape the character with two backslashes: \\.

[nacin]

In [21717]:

Merge [21205] and [21688] to the 3.4 branch. props SergeyBiryukov, yoavf. fixes #21590. see #21106.

[nacin]

After [21717], is there anything else necessary for the 3.4 branch?

SergeyBiryukov's comment suggests that this remaining issue has to do with jQuery 1.8.x, which is WP trunk only. The Sizzle selector engine was rebuilt in 1.8, so I would not be surprised if this did not affect 1.7. The original report does suggest this is a problem with 3.4.x. Does 21106.2.patch​ (or .3.patch) handle both?

[nacin]

The selectors in 21106.2.patch​ are a bit nasty, but look good to me.

We can actually chain in 21106.3.patch​, rather than relying on individual $el.find('a, input') calls. That would probably be more performant than .2.patch and lessens the risk in the 3.4 branch.

[SergeyBiryukov]

Replying to nacin:

After [21717], is there anything else necessary for the 3.4 branch?

I think so. In 3.4.1, the lagging (~3 seconds) only appears in IE when checking terms of a taxonomy starting with "even" (as described in the ticket). It would be good to fix that.

In trunk, the lagging appears in both IE and Firefox (~1 second in Firefox and IE 9, ~5 seconds in IE 8) when checking any category on a regular Edit Post screen as well. Additionally, a stack overflow error occurs in IE 8 when checking terms of a taxonomy starting with "even".

Upon more testing, it turned out that 21106.2.patch​ doesn't work as intended (the functions are not binded properly for some reason), so I've decided to simplify it.

21106.5.patch​ fixes the lagging in IE in 3.4.1. In trunk, it also fixes stack overflow in IE 8 and at least partially fixes the lagging (~0.2 seconds, almost unnoticeable, in FF, ~1 second in IE). We could look into further optimization in a new ticket.

[SergeyBiryukov]

Actually, checking categories is not the only slow thing in trunk.

With jQuery 1.8.1, most of the actions on Add New Post screen are extremely slow in IE 8: editing post title, changing post format, etc. This can also be seen on
​lessbloat's video (User 6) at 9:30.

Still, 21106.5.patch​ fixes that.

[azaozz]

Looked at this too. ​This part of wp-lists.js is pretty bad.

1. The process() runs on load with no arg, then on adding elements with the element as arg. This isn't needed if the events are delegated (i.e. attached to $(document) so they will trigger when new elements are added with ajax). Also, on load $el = $(document) so doing $el.find(...) goes through the whole DOM.

For this to work it shouldn't use .find() and only attach the events to $(document) on load. There's no need to run it on each ajax response. All this can be seen in action on the Comments screen.

2. We hit a bug in jQuery. When a class name contains :event it cannot be used as selector in jQuery even when the : is escaped (it won't select any elements). This affects process() too.

[nacin]

In [21737]:

Modify a few jQuery selectors to prevent overflows and lagging. Todo, rewrite wp-lists and improve all of these selectors.

props SergeyBiryukov. fixes #21106 for trunk. see #21152.

[nacin]

In [21738]:

Modify a few jQuery selectors to prevent overflows and lagging. props SergeyBiryukov. fixes #21106. merges [21737] to the 3.4 branch.

[nacin]

The rest of this can be handled in #21152.

[azaozz]

Replying to azaozz:

2. We hit a bug in jQuery. When a class name contains :event it cannot be used as selector in jQuery even when the : is escaped...

For the record: this only happens when wp-lists.js is loaded. So it's not a jQuery bug, it's wp-lists bug :)