![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png){kind=logo}
Opened 11 years ago
Closed 11 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 |
|---|---|---|---|
| Priority: | normal | Milestone: | 4.4.1 |
| Component: | Comments | Version: | 4.4 |
| Severity: | normal | Keywords: | fixed-major |
| Cc: | Focuses: |
Description (last modified by )
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)
Change History (14)
#4
@
11 years ago
- Description modified (diff)
- Focuses docs removed
- Keywords needs-patch added
- Milestone Awaiting Review → 4.4.1
- Severity major → normal
- Version 4.0 → 4.4
Culprit: [35435].
The workaround is to switch to the user_id field instead of user_ID.
#5
@
11 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?
#8
@
11 years ago
- Keywords has-patch has-unit-tests added; needs-patch removed
- Status assigned → accepted
34997.diff fixes this and adds a test.
#10
@
11 years ago
- Keywords fixed-major added; has-patch has-unit-tests removed
- Resolution fixed
- Status closed → reopened
#12
in reply to: ↑ 11
;
follow-up:
↓ 13
@
11 years ago
- Resolution fixed
- Status closed → reopened
Replying to johnbillion:
In 36039:
@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
@
11 years ago
- Resolution → fixed
- Status reopened → 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.
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