Query var on non-public taxonomy remains boolean true since [35333]

[johnbillion]

Still trying to pin down the root cause of this one. Steps to reproduce:

1. Register a non-public custom taxonomy with the following code:

register_taxonomy( 'foo', 'post', array(
	'public'    => false,
	'query_var' => true,
) );

2. Call the following code on the front end of your site:

var_dump( get_taxonomies( array(
	'query_var' => 'bar',
), 'objects' ) );

Note that the output erroneously contains the foo taxonomy, despite its query var not matching. The taxonomy's query var remains as boolean true, whereas in <=4.3 it was converted to the taxonomy name (foo in this example).

The query var for a non-public taxonomy should probably be set to boolean false in this case.

This seems to have been introduced in [35333]. The problem does not occur in the admin area due to a tweak introduced in [35680].

[boonebgorges]

Thanks for the ticket, @johnbillion.

It was decided in [35333] that the best way to prevent non-public taxonomies from being queryable on the front-end was to prevent them from registering a query_var. This is similar to the strategy that's used for non-public post types.

IMO it's a little bit weird to combine query_var = true with public = false. That being said, this does look like a legitimate bug.

The root problem, as I see it, is that wp_list_filter() (via wp_filter_object_list()) uses non-strict comparison ==, so that true == 'bar'. https://core.trac.wordpress.org/browser/tags/4.4/src/wp-includes/functions.php?marks=3332#L3302 I understand why we can't switch to strict comparison here, but I wonder how much stuff would break if wp_list_filter() explicitly avoided comparing booleans and non-empty strings - it's hard to imagine a legitimate use for this.

It's probably unlikely that we can change the behavior of wp_list_filter() or even get_taxonomies() here. So maybe a more local fix is that, in register_taxonomy(), non-public taxonomies have query_var forced to false. It's not very attractive, but it should minimize fallout. See 35089.diff​.

[johnbillion]

Replying to boonebgorges:

IMO it's a little bit weird to combine query_var = true with public = false.

Indeed it is. I only came across this bug because ​Jigoshop does this with its shop_order_status taxonomy.

[boonebgorges]

Replying to johnbillion:

Indeed it is. I only came across this bug because ​Jigoshop does this with its shop_order_status taxonomy.

Do you know off the top of your head why Jigoshop sets query_var = true? Will 35089.diff​ break something for them?

[boonebgorges]

Replying to boonebgorges:

Replying to johnbillion:

Indeed it is. I only came across this bug because ​Jigoshop does this with its shop_order_status taxonomy.

Do you know off the top of your head why Jigoshop sets query_var = true? Will 35089.diff​ break something for them?

I checked it out, and my guess is that Jigoshop does this only because of copy-and-pasting register_taxonomy() arguments. The plugin doesn't actually expect to see the query_var for non-public taxonomies anywhere.

I think that 35089.diff​ is the right move here: prior to 4.4, it was impossible for a query_var to be set to a boolean true, and it's true values that are causing the wp_list_filter() issues.

[boonebgorges]

In 36108:

Force non-public taxonomies to have a query_var of false.

[35333] implemented public=false for taxonomies. The implementation prevented
non-public taxonomies from having their archives accessed via query_var during
a normal request. But it didn't prevent non-public taxonomies from registering
their query vars in the $wp_taxonomies global. The latter implementation
details causes problems specifically when a taxonomy is registered with
query_var=true; for public taxonomies, register_taxonomy() translates this
into a query_var equivalent to the taxonomy name, but in the case of non-public
taxonomies, the query_var was set to the boolean itself. The boolean then
causes problems when using non-strict comparison to filter taxonomy objects by
query_var, as when using get_taxonomies().

This changeset addresses the issue by forcing the query_var property of
non-public taxonomies to false.

Fixes #35089.

[boonebgorges]

Reopening for 4.4.1.

[boonebgorges]

In 36109:

Force non-public taxonomies to have a query_var of false.

[35333] implemented public=false for taxonomies. The implementation prevented
non-public taxonomies from having their archives accessed via query_var during
a normal request. But it didn't prevent non-public taxonomies from registering
their query vars in the $wp_taxonomies global. The latter implementation
details causes problems specifically when a taxonomy is registered with
query_var=true; for public taxonomies, register_taxonomy() translates this
into a query_var equivalent to the taxonomy name, but in the case of non-public
taxonomies, the query_var was set to the boolean itself. The boolean then
causes problems when using non-strict comparison to filter taxonomy objects by
query_var, as when using get_taxonomies().

This changeset addresses the issue by forcing the query_var property of
non-public taxonomies to false.

Merges [36108] to the 4.4 branch.

Fixes #35089.