WordPress.org

Make WordPress Core

Opened 14 months ago

Last modified 10 months ago

#23469 new defect (bug)

Comment meta query is parsed before the pre_get_comments filter

Reported by: kasparsd Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.5
Component: Comments Keywords: has-patch
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 14 months ago.
Move the pre_get_comments filter before the meta_query parser

Download all attachments as: .zip

Change History (11)

kasparsd14 months ago

Move the pre_get_comments filter before the meta_query parser

comment:1 kasparsd14 months ago

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

comment:2 toscho14 months ago

  • Cc info@… added

comment:3 follow-up: SergeyBiryukov14 months 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.

comment:4 SergeyBiryukov14 months ago

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

comment:5 nacin14 months ago

Seems like WP_Query has opposite behavior.

comment:6 in reply to: ↑ 3 kasparsd14 months 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.

comment:7 SergeyBiryukov14 months ago

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

comment:8 follow-up: markjaquith12 months 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.

comment:9 in reply to: ↑ 8 kasparsd12 months 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.

comment:10 ryan10 months ago

  • Milestone changed from 3.6 to Future Release
Note: See TracTickets for help on using tickets.