Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion src/wp-includes/class-wp-term-query.php
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ class WP_Term_Query {
* @since 5.1.0 Introduced the 'meta_compare_key' parameter.
* @since 5.3.0 Introduced the 'meta_type_key' parameter.
* @since 6.4.0 Introduced the 'cache_results' parameter.
* @since 7.2.0 Introduced the 's' parameter as an alias of 'search'.
*
* @param string|array $query {
* Optional. Array or query string of term query parameters. Default empty.
Expand Down Expand Up @@ -158,6 +159,7 @@ class WP_Term_Query {
* (even if `$hide_empty` is set to true). Default true.
* @type string $search Search criteria to match terms. Will be SQL-formatted with
* wildcards before and after. Default empty.
* @type string $s Alias of `$search`. Default empty.
* @type string $name__like Retrieve terms with criteria by which a term is LIKE
* `$name__like`. Default empty.
* @type string $description__like Retrieve terms where the description is LIKE
Expand Down Expand Up @@ -211,6 +213,7 @@ public function __construct( $query = '' ) {
'term_taxonomy_id' => '',
'hierarchical' => true,
'search' => '',
's' => '',

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add @since for new variable.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 99ea3e2: added @since 7.1.0 Introduced the 's' parameter as an alias of 'search'. to the constructor docblock, plus the @type string $s entry in the parameter list. Apologies for the delayed reply, the fix has been on the branch since May 10 but I never pinged the thread.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Comment by Claude Opus 5

Adding s to query_var_defaults carries two side effects beyond this diff, because that array is consumed by two other pieces of machinery.

1. get_terms() legacy-signature detection. The legacy ( $taxonomy, $args ) form is distinguished from the modern one by array_intersect_key( $term_query->query_var_defaults, (array) $_args ) (src/wp-includes/taxonomy.php:1357), with an empty intersection meaning "legacy". A new single-letter key now participates in that test. For a taxonomy registered under the name s, get_terms( 's' ) parses to array( 's' => '' ), which now intersects the defaults, so the call is no longer detected as legacy, taxonomy is never set, and terms from every taxonomy are returned instead of just that one. This is admittedly exotic, but it is a behavior change outside the diff and deserves at least a mention on the ticket.

2. Duplicate cache entries for equivalent queries. generate_cache_key() slices $args by array_keys( $this->query_var_defaults ), so s now forms part of the key. array( 'search' => 'foo' ) and array( 's' => 'foo' ) produce byte-identical SQL yet different cache keys, so each equivalent query stores and misses its own entry.

Unsetting s once it has been copied onto search — the same treatment generate_cache_key() already gives cache_results and update_term_meta_cache — would resolve the second point and keep the parsed query_vars canonical.

'name__like' => '',
'description__like' => '',
'pad_counts' => false,
Expand Down Expand Up @@ -261,6 +264,11 @@ public function parse_query( $query = '' ) {

$query = wp_parse_args( $query, $this->query_var_defaults );

// 's' is a shorthand alias for 'search', matching WP_Query convention.
if ( '' === $query['search'] && '' !== $query['s'] ) {
$query['search'] = $query['s'];
}
Comment on lines +267 to +270

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Comment by Claude Opus 5

The alias is resolved here, but the documented extension points run afterward and neither of them re-resolves it: pre_get_terms fires at line 391 and the get_terms_args filter at line 434, both after parse_query() has completed.

Once s is documented as an alias of search, the following becomes a natural thing to write:

add_action(
    'pre_get_terms',
    static function ( $query ) {
        $query->query_vars['s'] = $_GET['s'];
    }
);

Setting search in that callback works; setting s is silently discarded and an unfiltered result set is returned, which is difficult to diagnose from the outside. There is precedent for handling this: meta_query is deliberately reparsed at line 674, with the comment "Reparse meta_query query_vars, in case they were modified in a 'pre_get_terms' callback." Resolving the alias a second time after get_terms_args, or else documenting that s is honored only at parse time, would close the gap.


$query['number'] = absint( $query['number'] );
$query['offset'] = absint( $query['offset'] );

Expand Down Expand Up @@ -425,6 +433,11 @@ public function get_terms() {
*/
$args = apply_filters( 'get_terms_args', $args, $taxonomies );

// Re-resolve 's' alias in case it was modified in a 'pre_get_terms' callback or 'get_terms_args' filter.
if ( '' === $args['search'] && '' !== $args['s'] ) {
$args['search'] = $args['s'];
}

// Avoid the query if the queried parent/child_of term has no descendants.
$child_of = $args['child_of'];
$parent = $args['parent'];
Expand Down Expand Up @@ -1182,7 +1195,7 @@ protected function generate_cache_key( array $args, $sql ) {
// $args can be anything. Only use the args defined in defaults to compute the key.
$cache_args = wp_array_slice_assoc( $args, array_keys( $this->query_var_defaults ) );

unset( $cache_args['cache_results'], $cache_args['update_term_meta_cache'] );
unset( $cache_args['cache_results'], $cache_args['update_term_meta_cache'], $cache_args['s'] );

if ( 'count' !== $args['fields'] && 'all_with_object_id' !== $args['fields'] ) {
$cache_args['fields'] = 'all';
Expand Down
1 change: 1 addition & 0 deletions src/wp-includes/taxonomy.php
Original file line number Diff line number Diff line change
Expand Up @@ -1324,6 +1324,7 @@ function get_term_to_edit( $id, $taxonomy ) {
* @since 4.5.0 Changed the function signature so that the `$args` array can be provided as the first parameter.
* Introduced 'meta_key' and 'meta_value' parameters. Introduced the ability to order results by metadata.
* @since 4.8.0 Introduced 'suppress_filter' parameter.
* @since 7.2.0 Introduced the 's' parameter as an alias of 'search'.
*
* @param array|string $args Optional. Array or string of arguments. See {@see WP_Term_Query::__construct()}
* for information on accepted arguments. Default empty array.
Expand Down
158 changes: 158 additions & 0 deletions tests/phpunit/tests/term/query.php
Original file line number Diff line number Diff line change
Expand Up @@ -1252,4 +1252,162 @@ public function test_id_and_parent_fields_should_return_parent_ids_keyed_by_term
$q2 = new WP_Term_Query();
$this->assertSame( $expected, $q2->query( $query_args ), 'Second query is not of the expected form.' );
}

/**
* @ticket 51811
*
* @covers WP_Term_Query::parse_query
*/
public function test_s_param_is_alias_for_search() {
register_taxonomy( 'wptests_tax', 'post' );

$term = self::factory()->term->create(
array(
'taxonomy' => 'wptests_tax',
'name' => 'Unique Findable Term',
)
);

$q = new WP_Term_Query(
array(
'taxonomy' => 'wptests_tax',
's' => 'Unique Findable',
'hide_empty' => false,
'fields' => 'ids',
)
);

$this->assertContains( $term, $q->terms, 'The s parameter should work as an alias for search.' );
}

/**
* @ticket 51811
*
* @covers WP_Term_Query::parse_query
*/
public function test_search_takes_precedence_over_s() {
register_taxonomy( 'wptests_tax', 'post' );

$term = self::factory()->term->create(
array(
'taxonomy' => 'wptests_tax',
'name' => 'Precedence Test Term',
)
);

$q = new WP_Term_Query(
array(
'taxonomy' => 'wptests_tax',
'search' => 'Precedence Test',
's' => 'nonexistent',
'hide_empty' => false,
'fields' => 'ids',
)
);

$this->assertContains( $term, $q->terms, 'The search parameter should take precedence over s.' );
}

/**
* @ticket 51811
*
* @covers WP_Term_Query::parse_query
*/
public function test_s_param_returns_empty_when_no_match() {
register_taxonomy( 'wptests_tax', 'post' );

self::factory()->term->create(
array(
'taxonomy' => 'wptests_tax',
'name' => 'Existing Non Matching Term',
)
);

$q = new WP_Term_Query(
array(
'taxonomy' => 'wptests_tax',
's' => 'absolutelynonexistentterm',
'hide_empty' => false,
'fields' => 'ids',
)
);

$this->assertEmpty( $q->terms, 'The s parameter should return empty for non-matching terms.' );
}
Comment on lines +1316 to +1336

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Comment by Claude Opus 5

This test passes whether or not the feature works. wptests_tax is registered but no terms are created in it, so $q->terms comes back empty regardless of whether s is honored — reverting the entire patch leaves the assertion passing.

For the assertion to carry weight, a term whose name does not match absolutelynonexistentterm needs to exist in wptests_tax, so that an unhonored s — which falls through to "return everything" — actually fails the test.


/**
* @ticket 51811
*
* @covers WP_Term_Query::get_terms
*/
public function test_s_param_can_be_set_in_pre_get_terms() {
register_taxonomy( 'wptests_tax', 'post' );

$term1 = self::factory()->term->create(
array(
'taxonomy' => 'wptests_tax',
'name' => 'Apple Term',
)
);
self::factory()->term->create(
array(
'taxonomy' => 'wptests_tax',
'name' => 'Banana Term',
)
);

$callback = static function ( $query ) {
$query->query_vars['s'] = 'Apple';
};

add_action( 'pre_get_terms', $callback );

$q = new WP_Term_Query(
array(
'taxonomy' => 'wptests_tax',
'hide_empty' => false,
'fields' => 'ids',
)
);

remove_action( 'pre_get_terms', $callback );

$this->assertSame( array( $term1 ), $q->terms, "Setting 's' in pre_get_terms should filter terms." );
}

/**
* @ticket 51811
*
* @covers WP_Term_Query::generate_cache_key
*/
public function test_s_and_search_produce_identical_cache_key() {
register_taxonomy( 'wptests_tax', 'post' );

$q1 = new WP_Term_Query();
$q1->query_vars = array(
'taxonomy' => 'wptests_tax',
'search' => 'Match',
'cache_results' => true,
);
$q1->parse_query( $q1->query_vars );

$reflection = new ReflectionMethod( $q1, 'generate_cache_key' );
if ( PHP_VERSION_ID < 80100 ) {
$reflection->setAccessible( true );
}

$key1 = $reflection->invoke( $q1, $q1->query_vars, 'SELECT * FROM wp_terms' );

$q2 = new WP_Term_Query();
$q2->query_vars = array(
'taxonomy' => 'wptests_tax',
's' => 'Match',
'cache_results' => true,
);
$q2->parse_query( $q2->query_vars );

$key2 = $reflection->invoke( $q2, $q2->query_vars, 'SELECT * FROM wp_terms' );

$this->assertSame( $key1, $key2, 'Queries using search and s should produce identical cache keys.' );
}
}
Loading