WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#34997 closed defect (bug) (fixed)

preprocess_comment filter does not contain old user_ID field for user_id, instead it has new user_id field

Reported by: cookiesowns Owned by: johnbillion
Milestone: 4.4.1 Priority: normal
Severity: normal Version: 4.4
Component: Comments Keywords: fixed-major
Focuses: Cc:

Description (last modified by johnbillion)

in wp-includes/comment.php, line 1680, the old $commentdata['user_ID'] field is not being set.

As a result, all old filters using the old field name in preprocess_comment filter will not have the proper parameter set.

I'm not sure what to do in this case, but we should update the PHPDOC above on line 1656 as $user_ID technically doesn't exist anymore..

Should we set both fields when passing through to preprocess_comment or?

Attachments (1)

34997.diff (2.6 KB) - added by johnbillion 5 years ago.

Download all attachments as: .zip

Change History (14)

#1 @cookiesowns
5 years ago

  • Severity changed from normal to major

#2 @cookiesowns
5 years ago

It also appears the documentation for preprocess_comment is still using user_ID instead of user_id in the example as well.

https://codex.wordpress.org/Plugin_API/Filter_Reference/preprocess_comment

#3 @SergeyBiryukov
5 years ago

  • Keywords reporter-feedback added
  • Version changed from 4.4 to 4.0

Hi @cookiesowns, welcome to Trac!

This code has not changed since [28915], see #23231.

Could you provide the steps to reproduce the issue on a clean install?

#4 @johnbillion
5 years ago

  • Description modified (diff)
  • Focuses docs removed
  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 4.4.1
  • Severity changed from major to normal
  • Version changed from 4.0 to 4.4

Culprit: [35435].

The workaround is to switch to the user_id field instead of user_ID.

#5 @cookiesowns
5 years ago

Indeed.

Given this code here...

<?php

function filter_comments( $commentdata ) {

  // This will fail due to new change

  if($commentdata['user_ID']) {

    // Do something with user
  }

}


add_filter( 'preprocess_comment', 'filter_comments' );

That code above will break with the new code.

<?php

function filter_comments( $commentdata ) {

  // This is how we're supposed to filter comments now apparently..

  if($commentdata['user_id']) {

    // Do something with user
  }

}


add_filter( 'preprocess_comment', 'filter_comments' );

Documentation has not yet been changed for this simple syntax change.

I'm thinking due to backwards compatibility, we might want to send both user_ID and user_id ? Or should the documentation just be changed to account for this?

#6 @SergeyBiryukov
5 years ago

  • Keywords reporter-feedback removed

#7 @jorbin
5 years ago

  • Owner set to johnbillion
  • Status changed from new to assigned

@johnbillion
5 years ago

#8 @johnbillion
5 years ago

  • Keywords has-patch has-unit-tests added; needs-patch removed
  • Status changed from assigned to accepted

34997.diff fixes this and adds a test.

#9 @johnbillion
5 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 36038:

Comments: When a comment is submitted, ensure the user_ID element in the array that's passed to the preprocess_comment filter gets populated.

Fixes #34997

#10 @johnbillion
5 years ago

  • Keywords fixed-major added; has-patch has-unit-tests removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

#11 follow-up: @johnbillion
5 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 36039:

Comments: When a comment is submitted, ensure the user_ID element in the array that's passed to the preprocess_comment filter gets populated.

Merges [36038] to the 4.4 branch.

Fixes #34997

#12 in reply to: ↑ 11 ; follow-up: @cookiesowns
5 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Replying to johnbillion:

In 36039:

Comments: When a comment is submitted, ensure the user_ID element in the array that's passed to the preprocess_comment filter gets populated.

Merges [36038] to the 4.4 branch.

Fixes #34997

@johnbillion

I see in your test, you're looking for both user_ID and user_id. However in the patch for comment.php, you've changed user_id, back to user_ID.

Shouldn't this break the test?

Another thing is, what do we do for users that have already worked-around this change by looking for user_id, instead of user_ID?

#13 in reply to: ↑ 12 @johnbillion
5 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

Replying to cookiesowns:

I see in your test, you're looking for both user_ID and user_id. However in the patch for comment.php, you've changed user_id, back to user_ID.

Shouldn't this break the test?

The wp_new_comment() function populates the `user_id` element when `user_ID` is passed. Hence, both elements are present in the $commentdata array which gets passed to the preprocess_comment filter.

Note: See TracTickets for help on using tickets.