Skip to:

Opened 11 years ago

Closed 6 years ago

#5079 closed enhancement (fixed)

Modify hook for posting new activity comment

Reported by: digitalminion's profile digitalminion Owned by: djpaul's profile djpaul
Milestone: 3.0 Priority: normal
Severity: normal Version:
Component: Activity Keywords: needs-patch good-first-bug 2nd-opinion


Just wanted to put out an idea for modifying the hook used when creating new content for activity comments.

At the moment in the function bp_activity_new_comment found in bp-activity-functions.php,
an apply_filter is called on the content of the comment using the hook bp_activity_comment_content.

This same hook is used to apply filters when retrieving the comment content in bp_get_activity_comment_content in bp-activity-template.php

I ran into trouble with this, as I wanted to modify the contents of the comment on creation, but didn’t want to run the same function against the comments when they are loaded to be displayed. In particular I wanted to apply wp_kses to the content to strip anchor tags supplied by the user, but as it stands since the same hook is used when loading the content (after bp_get_activity_content hook filters are applied which includes the bp_activity_truncate_entry function), the system added [Read More] link is rendered as plain text.

I’m sure there are several ways around this involving removing filters etc, but it strikes me as odd that that for example activity and group updates have the hooks bp_activity_new_update_content and groups_activity_new_update_content respectively for modifying user input content but it isn’t the same for comments.

My proposal would be to modify the function bp_activity_new_comment found in bp-activity-functions.php to call a hook bp_activity_new_comment_content on the content instead of bp_activity_comment_content. That way there is a unique hook for comment content creation.

Just wanted to put it out there to the community to see if someone more familiar with the code base sees this as an inconsistency or if there is a reason for doing it this way.

Change History (7)

#1 @DJPaul
11 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release
  • Severity changed from minor to normal

#2 @slaFFik
7 years ago

  • Keywords good-first-bug added

#3 @tw2113
7 years ago

  • Keywords 2nd-opinion added
  • Milestone changed from Future Release to 2.9

I think instead of trying to create new filters that add overhead and spots of maintenance, as little as it may be, it'd be better to pass a 2nd parameter to both for context. I'm open to suggestions for wording, but perhaps for bp_activity_new_comment we pass in a 2nd parameter of "new" and for bp_get_activity_comment_content we pass in a 2nd parameter of "display".

This will present a lot less headache, and would help developers conditionally modify the content.

#4 @hnla
7 years ago

  • Milestone changed from 2.9 to 3.0

#5 @DJPaul
6 years ago

Adding a new filter won't stop the reporter's problem of the same callback being triggered in the wrong place.
It would be nice to replace the filter, but we can't guarantee that won't negatively affect some existing sites.

#6 @DJPaul
6 years ago

I am very sorry it's taken the project five years to fix this issue. Thanks for reporting it way back when, @digitalminion! :)

#7 @djpaul
6 years ago

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

In 11837:

Activity: add new argument to duplicated bp_activity_comment_content filter.

This filter is unfortunately used in two places (bp_activity_new_comment() and bp_get_activity_comment_content()).
The new parameter should help 3rd party developers where the filter is called from.

Fixes #5079

Props tw2113

Note: See TracTickets for help on using tickets.