Skip to:
Content

BuddyPress.org

Opened 5 years ago

Last modified 5 years ago

#6068 new defect (bug)

Use standard event names instead of "bp_before_registration_submit_buttons" to make registration work with plugins

Reported by: elrata_ Owned by:
Milestone: Awaiting Contributions Priority: normal
Severity: normal Version:
Component: Core Keywords: needs-patch
Cc:

Description

Hi!

I'm using buddypress 2.1 and several captcha plugins doesn't work. For example wp-recaptcha:

https://wordpress.org/plugins/wp-recaptcha/

But the reason it is not working seems to be just because the callbacks are being added to different events. The registration form event is called differently when using Buddypress and this seems to break the captcha there (it works fine with the comments).

I did a simple patch, as a proof of concept, to wp-recaptcha to use the buddypress event names, and it works fine. You can see the patch here:

https://github.com/rata/wp-recaptcha-bp/commit/00c8a2441a2287f3b84303d0bbdc16551c55e5ad

(I also added the captcha to the lost password form, but ignore that)

Is it possible to use the standard wp event names, so plugins using the "add_action()" function work fine with Buddypress too ?

Thanks a lot,
Rodrigo

Change History (6)

#1 follow-up: @boonebgorges
5 years ago

In theory, I'd love to be able to fire WP's hooks during our registration process, but practically this might be difficult because of the number of plugins that have already added specific support for BP's hooks. In those cases, double validation and double markup would appear.

Can you be specific about the hooks that need to be fired for full parity? Looking at your changeset, it looks like 'register_form' is the front-end one. I don't see at a glance where the recaptcha plugin is validating on the back-end. Can you clarify?

#2 in reply to: ↑ 1 @elrata_
5 years ago

Replying to boonebgorges:

In theory, I'd love to be able to fire WP's hooks during our registration process, but > practically this might be difficult because of the number of plugins that have already added specific support for BP's hooks. In those cases, double validation and double markup would appear.

Ohh, okay, so is this an "historic mistake" I guess. Maybe it can be fixed in a when releasing a new major version of BP ?

I mean, because having to live for ever like this is kind of a PITA. Tons of plugins will simply not work because of this. And if there is no good reason to keep it, it's probably easier to fix the created plugins for BP than to teach all plguins, created now and in the future a user might want to use, to work with BP.

Can you be specific about the hooks that need to be fired for full parity? Looking at your changeset, it looks like 'register_form' is the front-end one. I don't see at a glance where the recaptcha plugin is validating on the back-end. Can you clarify?

In the very next line. Here:

https://github.com/rata/wp-recaptcha-bp/commit/00c8a2441a2287f3b84303d0bbdc16551c55e5ad#diff-fce6d8dd01a499c3994d022f1e669da5R71

It does an "add_action()" with 'bp_signup_validate' as "event".

Thanks for the quick reply!
Rodrigo

#3 follow-up: @boonebgorges
5 years ago

I mean, because having to live for ever like this is kind of a PITA. Tons of plugins will simply not work because of this. And if there is no good reason to keep it, it's probably easier to fix the created plugins for BP than to teach all plguins, created now and in the future a user might want to use, to work with BP.

We have a strong committment to backward compatibility. Breaking thousands of existing sites for the sake of a slightly improved developer experience is not how we operate. That being said, if there were a way to fix this without breaking things (which there might be), it's something I'd be glad to look at.

It does an "add_action()" with 'bp_signup_validate' as "event".

I see where you're hooking to BP actions, but it's not obvious at a glance which piece of functionality you're reproducing in the wp-recaptcha plugin. The only other hook that gets the check_recaptcha_generic callback is lostpassword_post, which clearly is not a registration-specific hook. How does the plugin validate WP registrations via wp-signup.php?

#4 in reply to: ↑ 3 @elrata_
5 years ago

Replying to boonebgorges:

I mean, because having to live for ever like this is kind of a PITA. Tons of plugins will simply not work because of this. And if there is no good reason to keep it, it's probably easier to fix the created plugins for BP than to teach all plguins, created now and in the future a user might want to use, to work with BP.

We have a strong committment to backward compatibility. Breaking thousands of existing sites for the sake of a slightly improved developer experience is not how we operate.

This is wrong. It's not for "improved developer experience" but it is for improved user experience. People who install plguins with BP and don't work are users IMHO.

That being said, if there were a way to fix this without breaking things (which there might be), it's something I'd be glad to look at.

I'm not into WP plugins, but something probably can be done. Even a BP version checking (like if bp_version >= X, then don't add the BP specific hooks). Or, of course, something nicer that WP allows. I really don't know about WP plugins. I leave that to you :)

It does an "add_action()" with 'bp_signup_validate' as "event".

I see where you're hooking to BP actions, but it's not obvious at a glance which piece of functionality you're reproducing in the wp-recaptcha plugin. The only other hook that gets the check_recaptcha_generic callback is lostpassword_post, which clearly is not a registration-specific hook. How does the plugin validate WP registrations via wp-signup.php?

Yeah, the other using it is that one because I added a hook for the lost_password form too. Now the recaptcha is shown on th BP registration page AND in the lost password form when using that patch.

And the check_recaptcha_generic is kind of hack based on check_comment function that I did (that's why it is introduced in the same patch. check_recaptcha_generic is not a pre-existing function). And as that function does the wp_die() thing, it is aborted when the function is called (i.e on POST) and the check fails.

But that function is not really relevant, there might be a better way to do it for sure. That was just a quick hack I did to prove that the callbacks are the problem.

#5 @boonebgorges
5 years ago

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

Thanks for the feedback. I think the idea here has merit, but it will take a fair bit of research (ideally, testing against popular plugins like wp-recaptcha) to be confident implementing it.

#6 @elrata_
5 years ago

Let me know if I can help you testing!

Note: See TracTickets for help on using tickets.