Compose AP comment-type exclusion with other plugins' filters#3315
Compose AP comment-type exclusion with other plugins' filters#3315pfefferle wants to merge 4 commits into
Conversation
The previous `comment_query` filter bailed as soon as any of `type__in`, `type` or `type__not_in` was set on the query — even when the caller had nothing to do with our comment types. That let ActivityPub Likes, Reposts, and Quotes leak into the front-end comment list whenever another plugin (e.g. GatherPress, which sets `type__in` for its own RSVP filtering) touched those query vars. The same flaw applied to themes setting `type__not_in` for their own exclusions: the AP filter abdicated and AP comment types showed up anyway. Now we only bail when the caller is explicitly requesting an AP comment type. Otherwise we merge our slugs into `type__not_in` on top of whatever the caller already had, so AP's exclusion composes with everyone else's filters. WP_Comment_Query already excludes the 'note' comment type by default since WP 6.9, so the merged result respects that too. Fixes #3306, fixes #2849.
There was a problem hiding this comment.
Pull request overview
Fixes regressions where the ActivityPub pre_get_comments handler bailed out as soon as any of type__in, type, or type__not_in was set on a comment query, allowing AP Likes/Reposts/Quotes to leak into front-end comment lists when other plugins/themes (e.g., GatherPress) used those query vars. The handler now merges AP comment-type slugs into type__not_in while still respecting callers that explicitly request an AP type.
Changes:
- Rewrote
Comment::comment_query()to merge AP slugs intotype__not_inrather than bailing, with an escape hatch whentype__in/typeexplicitly names an AP type. - Added three PHPUnit tests covering merge-with-
type__in, merge-with-type__not_in, and explicit AP type requests. - Added a
patch/fixedchangelogger entry.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| includes/class-comment.php | Replaces early-bail logic with type-merge logic that composes with other plugins' filters. |
| tests/phpunit/tests/includes/class-test-comment.php | New tests for type__in merge, type__not_in merge, and explicit AP type request. |
| .github/changelog/fix-comment-query-type-coexistence | Changelog entry describing the front-end leak fix. |
WordPress treats `type => 'all'` and `type__in => array( 'all' )` as "include everything, even types we'd normally hide" — `WP_Comment_Query` itself uses that flag to skip its built-in `note` exclusion. Our front-end filter needs the same escape hatch, otherwise a caller asking for the full set still can't see ActivityPub Likes, Reposts, or Quotes.
|
@Menrath does that fix the issue!? |
I will test this in a minute! :) |
|
@pfefferle Could you rebase this PR on the lasted updates, my blog already is on 8.3.0 :) |
|
@Menrath done! |
|
Hmm, I just deployed this branch and still the |
|
So maybe it is a Gatherpress issue then, because they ignore the ActivityPub excludes? |
|
Can you try to call the query filter later in the queue? Maybe: \add_action( 'pre_get_comments', array( static::class, 'comment_query' ), 50 ); |
|
The other way round it worked: \add_action( 'pre_get_comments', array( static::class, 'comment_query' ), 5 );And in GatherPress: add_action( 'pre_get_comments', array( $this, 'exclude_rsvp_from_comment_query' ), 10 );
/**
* Exclude RSVP comments from a query.
*
* This method modifies the comment query to exclude comments of the RSVP type. It
* ensures that RSVP comments are not included in the query results by adjusting the
* comment types in the query variables.
*
* Note: The comment_type field is not currently indexed in WordPress core,
* which may impact query performance. See https://core.trac.wordpress.org/ticket/59488
*
* @since 1.0.0
*
* @param WP_Comment_Query $query Current instance of WP_Comment_Query (passed by reference).
* @return void
*/
public function exclude_rsvp_from_comment_query( WP_Comment_Query $query ) {
// Process 'type' query var.
$current_comment_types = $query->query_vars['type'];
if ( ! empty( $current_comment_types ) ) {
if ( is_array( $current_comment_types ) ) {
$current_comment_types = array_values(
array_diff( $current_comment_types, array( Rsvp::COMMENT_TYPE ) )
);
} elseif ( Rsvp::COMMENT_TYPE === $current_comment_types ) {
$current_comment_types = '';
}
} else {
// Get all registered comment types from the database (cached).
$current_comment_types = $this->get_all_comment_types();
$current_comment_types = array_values( array_diff( $current_comment_types, array( Rsvp::COMMENT_TYPE ) ) );
}
$query->query_vars['type'] = $current_comment_types;
// Process 'type__in' query var.
$current_comment_types_in = $query->query_vars['type__in'];
if ( ! empty( $current_comment_types_in ) ) {
if ( is_array( $current_comment_types_in ) ) {
$current_comment_types_in = array_values(
array_diff( $current_comment_types_in, array( Rsvp::COMMENT_TYPE ) )
);
} elseif ( Rsvp::COMMENT_TYPE === $current_comment_types_in ) {
$current_comment_types_in = '';
}
$query->query_vars['type__in'] = $current_comment_types_in;
}
}CC @mauteri |
Fixes #3306. Also fixes #2849 (same root cause, different
query_varskey).Proposed changes
Comment::comment_query()now bails only when the caller explicitly requests an AP comment type viatype__in/type. In every other case it merges our slugs intotype__not_inon top of whatever the caller already set, so AP's exclusion composes with everyone else's filters instead of replacing or abdicating to them.Why
The previous logic bailed as soon as any of
type__in,type, ortype__not_inwas set on the query, even when the caller's intent had nothing to do with AP types. Two real-world cases broke as a result:type__infor its own RSVP filtering. AP's filter would seetype__inand bail; AP Likes/Reposts/Quotes then leaked into the front-end comment list. Example: https://event-federation.eu/2026/05/15/recurring-events-in-federation/.pre_get_commentsbreaks "manual"WP_Comment_Query->query()calls withtype__not_inparameters #2849 — A theme that setstype__not_infor its own exclusion (e.g.'type__not_in' => 'comment') would have it preserved, but AP types were no longer excluded. Closed previously by switching from "overwrite" to "bail", but bail still leaves AP types visible — the merge is the actual fix.The merge also continues to honor WP 6.9's built-in 'note' auto-exclusion in
WP_Comment_Query::parse_query_vars()— we don't have to add 'note' ourselves.Other information
Three new tests in
Test_Comment:test_comment_query_merges_ap_exclusion_with_existing_type_in— setstype__in => [ 'gatherpress_rsvp' ], assertstype__not_inends up containinglikeandrepost. This is the GatherPress regression.test_comment_query_merges_ap_exclusion_with_existing_type_not_in— setstype__not_in => [ 'pingback' ], asserts bothpingbackand the AP slugs are present after the filter. This is thepre_get_commentsbreaks "manual"WP_Comment_Query->query()calls withtype__not_inparameters #2849 regression.test_comment_query_respects_explicit_ap_type_request— setstype__in => [ 'like' ], asserts the filter does not pushlikeintotype__not_in. Escape hatch for callers who want AP comments back.All 70
Test_Commenttests pass locally (228 assertions).Testing instructions
trunk: AP Likes/Reposts/Quotes appear in the comment list.type__not_inin its comment template (thepre_get_commentsbreaks "manual"WP_Comment_Query->query()calls withtype__not_inparameters #2849 shape).Changelog entry
Already added at `.github/changelog/fix-comment-query-type-coexistence`.