Skip to:
Content

BuddyPress.org

Opened 3 years ago

Closed 3 years ago

#7318 closed defect (bug) (fixed)

Cleanup incorrect remove_action() & remove_filter() usage

Reported by: slaFFik Owned by: djpaul
Milestone: 2.8 Priority: low
Severity: normal Version:
Component: Core Keywords: good-first-bug needs-refresh has-patch
Cc:

Description (last modified by slaFFik)

WordPress functions remove_action() and remove_filter() have 3 params:

function remove_action( $tag, $function_to_remove, $priority = 10 ) {}
function remove_filter( $tag, $function_to_remove, $priority = 10 ) {}

While in some places in BuddyPress core they are called with 4 params, like this:

remove_action( 'bp_activity_before_save', 'bp_activity_check_moderation_keys', 2, 1 );
remove_filter( 'bp_activity_generate_action_string', $actions->{$activity->component}->{$activity->type}['format_callback'], 10, 2 );

The 4th param should be removed as useless.

Attachments (2)

patch.patch (11.9 KB) - added by ketuchetan 3 years ago.
Added patch for the remove_filter and remove_Action
7318.patch (10.0 KB) - added by ketuchetan 3 years ago.
Modified the patch.

Download all attachments as: .zip

Change History (11)

#1 @slaFFik
3 years ago

  • Description modified (diff)
  • Summary changed from Cleanup incorrect remove_action() usage to Cleanup incorrect remove_action() & remove_filter() usage

#2 @boonebgorges
3 years ago

  • Keywords needs-patch added
  • Priority changed from normal to low

remove_action() and remove_filter() formally accepted a fourth parameter until 4.7; it's only with the introduction of WP_Hook that the function signature changed. But prior to 4.7, the parameter was unused, so it does no harm to remove it.

#3 @ketuchetan
3 years ago

  • Cc ketuchetan@… added
  • Keywords has-patch added; needs-patch removed

Hi @boonebgorges & @slaFFik

I have removed the fourth parameter everywhere on the plugin and I have added my patch.

I have also checked the plugin working flow and every where working fine on my end.

Once, you get the time please review it and let me know if it is fine or not.

@ketuchetan
3 years ago

Added patch for the remove_filter and remove_Action

This ticket was mentioned in Slack in #buddypress by chetansatasiya. View the logs.


3 years ago

#5 @slaFFik
3 years ago

  • Keywords needs-refresh added

@ketuchetan
Thanks for the patch!

Could you please make sure that you are not modifying a 3rd-party plugin, bundled with BuddyPress - bp-forums? It's an external source of code, and should not be directly modified.

Also, would be really nice to have the patch created from the root directory of the repository, that means that all paths to BuddyPress PHP files should be with /src/ and the beginning. Currently you created a patch from inside of /build/ directory.

#6 @ketuchetan
3 years ago

  • Cc ketuchetan@… removed
  • Keywords needs-refresh removed

Hi @slaFFik

Thanks for the guidance and help to submit my first patch on the BuddyPress. Now, I have added my new patch as per your above instructions.

Please check it out and let me know your thoughts in this.

@ketuchetan
3 years ago

Modified the patch.

This ticket was mentioned in Slack in #buddypress by chetansatasiya. View the logs.


3 years ago

#8 @slaFFik
3 years ago

  • Keywords needs-refresh added

@ketuchetan
I checked your patch and saw that you missed several remove_action() and remove_filter() calls. On a first glance (actually, search in PhpStorm) I noticed 4 results:

remove_action( 'bp_send_email_success', 'bp_core_deprecated_email_actions', 20, 2 );
remove_action( 'bp_user_query_populate_extras', array( $this, 'populate_group_member_extras' ), 10, 2 );
remove_filter( 'bp_get_group_type_directory_permalink', array( $this, 'group_type_permalink_use_admin_filter' ), 10, 2 );
remove_filter( "bp_get_options_nav_{$css_id}", 'bp_group_admin_tabs_backcompat', 10, 3 );

Can you please update them too?

#9 @djpaul
3 years ago

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

In 11256:

Fix incorrect remove_action and remove_filter usage for WordPress 4.7.

remove_action and remove_filter formally accepted a fourth parameter until WP 4.7; it's only with the introduction of the WP_Hook class that the function signature changed. Prior to WP 4.7, the fourth parameter was unused, so it does no harm to remove it and tidy things up.

Fixes #7318

Props ketuchetan

Note: See TracTickets for help on using tickets.