WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#35337 closed defect (bug) (wontfix)

Global $post is null in "comment_post" and "pre_comment_on_post" hooks

Reported by: dennis_f Owned by: johnbillion
Milestone: Priority: normal
Severity: normal Version: 4.4
Component: Comments Keywords: 2nd-opinion
Focuses: Cc:

Description

Since 4.4 when using the "comment_post" and "pre_comment_on_post" hooks, the global $post is null, you can test this with the following code:

<?php
add_action ('comment_post', 'my_comment_post_func', 10, 2);
function my_comment_post_func($comment_ID, $comment_approved ){
        global $post;
        // $post is NULL here
}

I tested the same code snippet with 4.3 and $post is set correctly in there

Change History (7)

#1 follow-up: @boonebgorges
4 years ago

@dennis_f In what context is comment_post firing? Is it when submitting a comment via wp-comment-post.php? Or for all uses of wp_new_comment()? (eg via AJAX in the Dashboard)

#2 in reply to: ↑ 1 @dennis_f
4 years ago

Replying to boonebgorges:

@dennis_f In what context is comment_post firing? Is it when submitting a comment via wp-comment-post.php? Or for all uses of wp_new_comment()? (eg via AJAX in the Dashboard)

This happened when posting a new comment to a blog post, just by using the standard post comment form on the front-end - I haven't tested it with other uses. I tested it with different themes (including TwentyFifteen and TwentySixteen), always with the same result.

Last edited 4 years ago by dennis_f (previous) (diff)

#3 @boonebgorges
4 years ago

Gotcha. It appears that this is due to [34799], which moved the handling of comment submission from wp-comments-post.php into a new function, wp_handle_comment_submission(). This move had the side-effect that a bunch of the variables that were previously being declared in the global scope - such as $post - are now local to the function.

Technically, each of these deglobalized variables is a potential backward compatibility break. It's possible, for example, that someone is filtering 'comment_post' and expecting $GLOBALS['status_obj'] to be populated. (To quote @wonderboymusic: Globals will kill WordPress. Globals are killing WordPress.) In most of these cases, I think it's perfectly reasonable for us to drop backward compatibility. But $post may be a special case, since we have a convention throughout WordPress for global $post to contain the "current" post object. So maybe we can make an exception here and hoist the post into global scope.

Ping @johnbillion @SergeyBiryukov

#4 @johnbillion
4 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 4.4.2
  • Owner set to johnbillion
  • Status changed from new to accepted

#5 follow-up: @johnbillion
4 years ago

  • Keywords 2nd-opinion added; needs-patch removed

It's worth noting that relying on the global $post object during the comment_post filter is not reliable anyway, because it's not globalised when a comment is submitted via an AJAX reply in the admin area.

For the above reason, I'm inclined to wontfix this, unless I'm mistaken or there's another reason to globalise $post or any other variables inside wp_handle_comment_submission().

#6 in reply to: ↑ 5 @boonebgorges
4 years ago

Replying to johnbillion:

It's worth noting that relying on the global $post object during the comment_post filter is not reliable anyway, because it's not globalised when a comment is submitted via an AJAX reply in the admin area.

For the above reason, I'm inclined to wontfix this, unless I'm mistaken or there's another reason to globalise $post or any other variables inside wp_handle_comment_submission().

The point about the AJAX handler is pretty persuasive. wontfix seems OK to me on those grounds.

#7 @johnbillion
4 years ago

  • Milestone 4.4.2 deleted
  • Resolution set to wontfix
  • Status changed from accepted to closed

As above. Feel free to re-open if there's a valid reason to globalise $post here.

Note: See TracTickets for help on using tickets.