Make WordPress Core

Opened 7 years ago

Closed 7 years ago

#40286 closed defect (bug) (fixed)

It appears to me that comments_open() is passing the wrong args to apply_filters

Reported by: sringwood's profile sringwood Owned by: rachelbaker's profile rachelbaker
Milestone: 4.8 Priority: normal
Severity: normal Version: 1.5
Component: Comments Keywords: has-patch commit
Focuses: Cc:

Description

The function comments_open() is passed one argument which may be null ($post_id) which is used to call get_post(). The function get_post() will use the current post if $post_id is null.

The last line of the function comments_open() is

return apply_filters( 'comments_open', $open, $post_id );

Logically in the call it seems to me that $post_id (which may be null) should be $_post->ID to reflect the post returned by the call get_post().

Attachments (4)

40286.patch (895 bytes) - added by shulard 7 years ago.
40286.2.patch (895 bytes) - added by shulard 7 years ago.
Mistake in diff file, the first one will remove my updates…
40286-with-docs.patch (1.2 KB) - added by shulard 7 years ago.
Fix the code and update the filters docs.
40286.3.diff (1.2 KB) - added by rachelbaker 7 years ago.
Adds conditional to use 0 if $_post->ID is undefined

Download all attachments as: .zip

Change History (16)

#1 @johnbillion
7 years ago

  • Keywords needs-patch good-first-bug added
  • Milestone changed from Awaiting Review to Future Release
  • Version changed from 4.7.3 to 1.5

Hi @sringwood, welcome to WordPress Trac!

Well spotted, this does appear to be a valid bug. It affects the pings_open() function too.

@shulard
7 years ago

#3 @shulard
7 years ago

Hello guys,

I've just submitted a patch for that little change !

@shulard
7 years ago

Mistake in diff file, the first one will remove my updates...

#4 @johnbillion
7 years ago

  • Keywords has-patch needs-testing added; needs-patch removed
  • Milestone changed from Future Release to 4.8

Thank you, @shulard!

#5 follow-up: @rachelbaker
7 years ago

  • Keywords needs-docs added

Looks like this also needs a change to the inline docs for the comments_open and pings_open filters since $_post->ID would always be an integer, and never a WP_Post object.

@shulard
7 years ago

Fix the code and update the filters docs.

#6 in reply to: ↑ 5 @shulard
7 years ago

@rachelbaker, you are right, I've submitted a new patch with fixed doc too.

@rachelbaker
7 years ago

Adds conditional to use 0 if $_post->ID is undefined

#7 @rachelbaker
7 years ago

  • Keywords 2nd-opinion added; good-first-bug needs-testing needs-docs removed

This might be overkill, but because there is no way to be sure $_post->ID is defined, I added a conditional that will pass 0 to the comments_open() or pings_open() filter for the Post ID. Would love a second opinion on if this conditional is worthy of the edge-case handling. 40286.3.diff

#8 @shulard
7 years ago

I though that get_post function will always return an WP_Post instance so $_post->ID will always be defined. It can be null but it's defined ($ID is a public property of WP_Post class).

Last edited 7 years ago by shulard (previous) (diff)

#9 @rachelbaker
7 years ago

@shulard get_post() can return null. These two functions can be used anywhere, and cannot assume the post global is populated.

#10 @rachelbaker
7 years ago

  • Keywords 2nd-opinion removed

#11 @rachelbaker
7 years ago

  • Keywords commit added

Marking for commit since the change here is also in line with the patches on #40143.

#12 @rachelbaker
7 years ago

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

In 40666:

Comments: Correct the $post_id parameter passed to the 'comments_open' and 'pings_open' filters.

Fixes bug where previously the $post_id function argument was passed to the '_open' filters, instead of the result of the get_post() call. If the current post is not found, the $post_id filter parameter will be 0.

Props johnbillion, shulard, rachelbaker.
Fixes #40286.

Note: See TracTickets for help on using tickets.