WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 2 years ago

Last modified 2 years ago

#23469 closed defect (bug) (duplicate)

Comment meta query is parsed before the pre_get_comments filter

Reported by: kasparsd Owned by: chriscct7
Milestone: Priority: normal
Severity: normal Version: 3.5
Component: Comments Keywords: needs-patch needs-refresh
Focuses: Cc:

Description

Currently it is impossible to use the pre_get_comments filter to adjust the meta_query, because parse_query_vars( $this->query_vars ) is run before the filter is applied http://core.trac.wordpress.org/browser/tags/3.5.1/wp-includes/comment.php#L243:

// Parse meta query
$this->meta_query = new WP_Meta_Query();
$this->meta_query->parse_query_vars( $this->query_vars );

do_action_ref_array( 'pre_get_comments', array( &$this ) );

Comment meta query should be parsed after the pre_get_comments filter has been applied.

Attachments (1)

pre_get_comments-before_meta_parse.diff (650 bytes) - added by kasparsd 5 years ago.
Move the pre_get_comments filter before the meta_query parser

Download all attachments as: .zip

Change History (14)

@kasparsd
5 years ago

Move the pre_get_comments filter before the meta_query parser

#1 @kasparsd
5 years ago

  • Cc kaspars@… added
  • Keywords has-patch added; needs-patch removed

#2 @toscho
5 years ago

  • Cc info@… added

#3 follow-up: @SergeyBiryukov
5 years ago

  • Version changed from 3.5.1 to 3.5

Related: [22074] (for #21003).

Unless I'm missing something, the hook appears to be at the correct location. parse_query_vars() can be run again from the hooked function if needed.

The suggested patch would make $this->meta_query object inaccessible for the hooked functions.

#4 @SergeyBiryukov
5 years ago

  • Keywords reporter-feedback added
  • Severity changed from major to normal

#5 @nacin
5 years ago

Seems like WP_Query has opposite behavior.

#6 in reply to: ↑ 3 @kasparsd
5 years ago

Replying to nacin:

Seems like WP_Query has opposite behavior.

WP_Query parses the meta query after the 'pre_get_posts' gets applied: http://core.trac.wordpress.org/browser/tags/3.5.1/wp-includes/query.php#L1934. And it makes sense, because you would need to manually run $query->meta_query->parse_query_vars( $query->query_vars ); every time you modify something related to the meta query.

Replying to SergeyBiryukov:

The suggested patch would make $this->meta_query object inaccessible for the hooked functions.

Exactly, just like WP_Query doesn't make the parsed meta_query available on pre_get_posts.

In your pre_get_posts filters you don't have to manually parse_query to actually apply the changes, so it shouldn't be required for changes to meta_query either.

#7 @SergeyBiryukov
5 years ago

  • Keywords reporter-feedback removed
  • Milestone changed from Awaiting Review to 3.6

#8 follow-up: @markjaquith
4 years ago

This was odd to me. https://gist.github.com/markjaquith/681af58ce22d79c08c09

But moving it would be a breaking change. Maybe we should just have another hook that happens before parsing.

#9 in reply to: ↑ 8 @kasparsd
4 years ago

But moving it would be a breaking change.

I think parse_query_vars for meta_query already takes care of double parsing, in case somebody is doing it manually on pre_get_comments.

Maybe we should just have another hook that happens before parsing.

That could be a solution, but it would make the pre_get_ filters inconsistent.

#10 @ryan
4 years ago

  • Milestone changed from 3.6 to Future Release

#11 @chriscct7
2 years ago

  • Keywords needs-patch needs-refresh added; has-patch removed
  • Milestone changed from Future Release to 4.4
  • Owner set to chriscct7
  • Status changed from new to assigned

#12 @kasparsd
2 years ago

  • Resolution set to duplicate
  • Status changed from assigned to closed

This was fixed in #32762

#13 @netweb
2 years ago

  • Milestone 4.4 deleted
Note: See TracTickets for help on using tickets.