WordPress.org

Make WordPress Core

Opened 3 months ago

Last modified 3 months ago

#43521 reopened defect (bug)

Excluded terms filter in get_adjacent_post is not executed if excluded_terms parameter is empty

Reported by: zottto Owned by: SergeyBiryukov
Milestone: 5.0 Priority: normal
Severity: normal Version: 4.9.4
Component: Posts, Post Types Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

The filter get_previous_post_excluded_terms / get_next_post_excluded_terms is only executed if the parameter $excluded_terms of get_adjacent_post (resp. get_previous_post / get_next_post) is set. If $exclued_terms is not set (e.g. because the parameter is optional) it is not possible to change the list of excluded terms.

Expected behaviour: The filter should also be executed with an empty $exclided_terms list to be able to add terms to this list.

The apply_filter is called within a condition if ( $in_same_term || ! empty( $excluded_terms ) ) which is not met if the parameter is empty.

Attachments (4)

43521.diff (3.4 KB) - added by soulseekah 3 months ago.
Patch + tests
43521.1.diff (4.6 KB) - added by soulseekah 3 months ago.
43521.2.diff (2.0 KB) - added by soulseekah 3 months ago.
Move excluded_terms sanitization to before the filter
43521.3.diff (2.0 KB) - added by soulseekah 3 months ago.
Final + test

Download all attachments as: .zip

Change History (10)

@soulseekah
3 months ago

Patch + tests

#1 @soulseekah
3 months ago

  • Keywords has-patch has-unit-tests added

Patch + test. Should work ;)

#2 @SergeyBiryukov
3 months ago

  • Component changed from General to Posts, Post Types
  • Milestone changed from Awaiting Review to 5.0

#3 @SergeyBiryukov
3 months ago

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

In 42828:

Link Template: Apply get_{$adjacent}_post_excluded_terms filter to an empty excluded_terms parameter as well.

Props soulseekah, zottto.
Fixes #43521.

#4 @zottto
3 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Hi, thanks for fixing the bug so fast. But in my opinion there is now another change in the behaviour of the filter. Before the fix, the filter was applied only to an array of term IDs. Now the filter is applied before the backwards compatibility part that changes the old usage of term IDs concatenated with "and" to an array.

If the filter is executed now it could happen that the term IDs handled in the filter are in the old style which was not possible before.

From my point of view it would be better to apply the filter after that back-compat part (or to execute the back-compat part before the filter, whatever makes more sense).

@soulseekah
3 months ago

@soulseekah
3 months ago

Move excluded_terms sanitization to before the filter

#5 @soulseekah
3 months ago

Good catch @zottto 43521.2.diff​ additional patch that moves things up

@soulseekah
3 months ago

Final + test

#6 @soulseekah
3 months ago

43521.3.diff includes the test as well, making sure the filter does not error out when a string is passed in (implying that the block of code that transforms the string into the array was run).

Thanks.

Note: See TracTickets for help on using tickets.