Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#38589 closed defect (bug) (fixed)

hook 'get_terms_args' calls two times ...

Reported by: tkama's profile Tkama Owned by: boonebgorges's profile boonebgorges
Milestone: 4.7 Priority: normal
Severity: normal Version: 4.6
Component: Taxonomy Keywords:
Focuses: Cc:

Description

There is no logic situation in new WP_Term_Query class

Look into get_terms() method - it calls $this->parse_query( $this->query_vars );. When parse_query call hook 'get_terms_args' a after it get_terms() itself calls hook
'get_terms_args' one more time.

So in one request we have double hook calls...

It seems like bug!

link to class: https://developer.wordpress.org/reference/classes/wp_term_query/

Change History (5)

#1 @boonebgorges
6 years ago

  • Component changed from General to Taxonomy
  • Focuses administration removed
  • Milestone changed from Awaiting Review to 4.7
  • Version changed from 4.6.1 to 4.6

Hi @Tkama - Thank you for the ticket, and welcome to WordPress Trac!

You're correct that it's illogical to call the 'get_terms_args' filter twice. This problem was introduced in [37572] along with WP_Term_Query. I believe that the error can be explained like this: Prior to [37572], 'get_terms_args' had been applied in get_terms(), after the defaults had been set up. (This usage dates back many versions.) But parse_query() seems, in some abstract sense, like the "right" place for the filter to live, which is why I assume it was introduced in [37572].

I think the correct fix is to remove the 'get_terms_args' filter in WP_Term_Query::parse_query(), but I'd like a second opinion on that before moving forward. @flixos90, since you drafted the original patch, could I ask for your thoughts?

#2 @flixos90
6 years ago

@boonebgorges: I must have accidentally introduced it. I agree that the occurrence in WP_Term_Query::parse_query() should be removed, since the name of the filter indicates it is used when get_terms() is run and furthermore since other query classes don't have such a filter in their parse_query() method.

Good catch @Tkama!

#3 @boonebgorges
6 years ago

  • Owner set to boonebgorges
  • Resolution set to fixed
  • Status changed from new to closed

In 39057:

Taxonomy: Remove redundant 'get_terms_args' filter call from WP_Term_Query.

Introduced in [37572]. The correct 'get_terms_args' filter call is in
WP_Term_Query::get_terms().

Props Tkama.
Fixes #38589.

#4 @Offereins
6 years ago

If I understand correctly, since r39057, the query vars of WP_Term_Query cannot be modified when parsing them on a standalone query object (when not calling get_terms()), right? This is because the query object in the parse_term_query and pre_get_terms actions isn't provided by reference (which is the case in the similar hooks in WP_Query).

#5 @boonebgorges
6 years ago

Hi @Offereins - In contexts like the following, the query object is passed by reference as the default behavior in PHP 5+:

do_action( 'parse_term_query', $this );

Instances of &$this in the codebase are a remnant of PHP 4 support.

(We've already spoken about this over Slack, but I wanted to repeat it here for posterity :) )

Note: See TracTickets for help on using tickets.