WordPress.org

Make WordPress Core

Opened 9 months ago

Closed 9 months ago

Last modified 9 months ago

#24913 closed defect (bug) (invalid)

comment_text filter used differently in two places in core

Reported by: ronalfy Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.6
Component: Comments Keywords: dev-feedback
Focuses: Cc:

Description

The 'comment_text' filter is being used differently in two different places in core.

For the life of me, I can't figure out why.

The first instance is being used as a comment is posted in /wp-includes/comment.php, passing only one argument:

$comment = apply_filters( 'comment_text', $comment );

The 2nd instance is being used when a comment is displayed in /wp-includes/comment-template.php, passing two arguments.

echo apply_filters( 'comment_text', get_comment_text( $comment_ID ), $comment );

The problem is, if someone is hooked into the 'comment_text' filter and is expecting two arguments, the 2nd one won't be set when a comment is posted.

I recommend changing the filter name 'comment_text' when the comment is being posted.

Thoughts?

Change History (3)

comment:1 ericmann9 months ago

  • Keywords 2nd-opinion removed

Nope, not a bug. Two different contexts.

In the first (/wp-includes/comment.php), the $comment variable contains just the comment text. In the second (/wp-includes/comment-template.php), the $comment variable contains the actual comment object itself (based on the return of get_comment( $comment_ID )).

So the filters are filtering the same fields. The issue then becomes the number of parameters and that, too, is not a bug.

The first comment text filter is used specifically to filter the text of the comment before things like inserting it into the database. The second is used when filtering the text of the comment before displaying it on a page - this follows the pattern of not trusting data before inserting it in the DB and also not trusting the data when pulling it back out. Using the same filter here helps us use exactly the same rules to filter the content both pre and post DB.

If the problem is the number of fields, simply make the second parameter of your function optional:

function filter_text( $comment_text, $comment = null ) {
    // Do something to the comment, possibly switching based on the presence of $comment
}
add_filter( 'comment_text', 'filter_text', 10, 2 );

If you only want to do things when the comment is inserted into the DB, just verify that null === $comment in the function above. If you only want to do things when comments are displayed, verify that $null !== $comment.

IMO this isn't a bug but a pretty powerful differentiating factor between a filter that can be used in different places to do either the same or different things.

comment:2 ronalfy9 months ago

  • Resolution set to invalid
  • Status changed from new to closed

@ericmann,

Thanks for that explanation. I'll agree that it's not a bug. Your solution is exactly what I did to get rid of any pesky errors.

comment:3 DrewAPicture9 months ago

  • Milestone Awaiting Review deleted
Note: See TracTickets for help on using tickets.