Opened 13 years ago
Closed 8 years ago
#3625 closed enhancement (no action required)
Core functions should not hook to bp_init with default priority
Reported by: | boonebgorges | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 1.5 |
Component: | Core | Keywords: | needs-refresh |
Cc: |
Description
I had a good discussion on ticket #3622 about the bp_init hook and plugins. Conceptually, I think that the idea behind bp_init is to let plugins attach certain actions to a point in the load order when they can be sure that BP's core functionality has been fully loaded. This mostly works - bp_setup_globals, bp_setup_nav, and other initialization hooks all happen before bp_init priority 10. However, there are a few important places in BP where we use priority 10, with the result that if a plugin happens to be loaded by WordPress *before* BP is, those hooks could fail to find the corresponding action. To wit:
bp_core_load_admin_bar_css()
bp_core_load_buddybar_css()
bp_core_admin_menu_init() (initializes the admin menu loader)
bp_core_add_ajax_hook() (creates the wp_ajax_ hooks on the front-end that WP has by default on the back end)
bp_core_wpsignup_redirect() (redirects users away from wp-signup.php when BP is installed)
There are also some places in the friends component where we hook to bp_init, but IMO that's not as crucial, since our optional components essentially operate like plugins anyway.
I propose that all of the above function be moved from bp_init priority 10 to 8 or 9. This will obviously need some thorough testing (though I'm pretty certain that making these particular functions fire a bit *earlier* is not going to hurt anything). I'll post a patch.
Attachments (2)
Change History (14)
#2
@
13 years ago
I think setting it to 8 does hurt some things, for example the title is set at priority 9.
http://buddypress.trac.wordpress.org/browser/trunk/bp-core/bp-core-hooks.php#L33
I don't think that hurts anything.
Even if a plugin hooks to bp_init with priority 1, BuddyPress should be fully initialised at that point.
Maybe. But that will require a pretty massive overhaul of load order. Our entire initialization routine runs on bp_init.
#3
@
13 years ago
Sorry but your patch doesn't fixes the problem.
BuddyPress should support add_action('wp_ajax_') on the default priority 10 of bp_init.
Lowering the priority from the bp_core_add_ajax_hook() function that calls do_action('wp_ajax_') from 10 to 8 only makes it worse.
#4
@
13 years ago
Sorry but your patch doesn't fixes the problem.
You may be right. I wasn't able to reproduce the problem in the first place, so I was kinda guessing.
Lowering the priority from the bp_core_add_ajax_hook() function that calls do_action('wp_ajax_') from 10 to 8 only makes it worse.
I don't understand how it could "make it worse". I guess I assumed that the problem was that, by hooking to bp_init with priority 10, you were sometimes attempting to hook to wp_ajax_ before BP was there to listen for the hooks. Moving the priority up would mean that the wp_ajax_ hook mechanism would be in place by the time you tried adding your own stuff at bp_init:10. It's possible that I'm misdiagnosing the issue, but in any case I don't know how it would make it worse, since it either works or it doesn't work - there's not really room for "better" or "worse" here.
If you have ideas about how this issue could be solved in a broader way, I would welcome a patch.
#5
@
13 years ago
You probably can't reproduce because I load plugins from /mu-plugins.
/mu-plugins are always loaded before /plugins/buddypress
Your patch makes this a little worse, because if you move the ajax function to bp_init priority=8, it also breaks (plugins hooked to bp_init with default priority=10) when BuddyPress "IS" loaded before the actual plugin.
#6
@
13 years ago
In that case, I guess I don't really understand the problem at all. I thought the issue was that BP core functions were loading too *late*. But you're saying that they load too early?
#7
@
13 years ago
I'm a little confused myself, my previous reply was incorrect, sorry.
Your patch makes this a little worse, because if you move the ajax function to bp_init priority=8, it also breaks (plugins hooked to bp_init with default priority=10) when BuddyPress "IS" loaded before the actual plugin.
My wp_ajax plugin is only activated for the rootblog, so it's in /plugins instead.
BuddyPress is actually loaded "before" this plugin.
That's why BuddyPress calls do_action('wp_ajax_') before my plugin has called add_action('wp_ajax_') when they are both hooked to bp_init with the default priority 10.
So yes, these BP core actions load too early, hooking into bp_setup_globals is the only way to get the ajax call working (for now).
---
I thought the issue was that BP core functions were loading too *late*.
That was related to the other ticket when a plugin hooks to init from /mu-plugins with default priority 10.
At that point bp_init is not called (yet), so you can't get activity slugs or other BuddyPress globals.
Hooking to bp_init instead of init already fixes that problem.
However when a developer only tests on /plugins and BuddyPress is loaded before the plugin, they won't have problems hooking into init. (until a multisite user drops their plugin into /mu-plugins)
#8
@
13 years ago
Just an idea, not tested.
Plugins already hooked to bp_init (with default/10 or higher priority) will not be affected.
#9
@
13 years ago
This is already confusing, but I just wanted to jump in too see if I can bring some clarity. First, while I don't think it's necessary, it definitely wouldn't hurt to load our more important business functions, like the ones listed by boone, a little earlier for those hooking into bp_init priority 10.
Second, the biggest challenge with hooks is knowing when/where to hook. In the case of the wp_ajax_ hook example, it seems webraket is trying something like this
<?php class Webraket_Class { public function __construct(){ add_action('wp_ajax_', array(&$this,'my_wp_ajax_')); } function my_wp_ajax_() { // } } function Init_Webraket_Class(){ $webraket = new Webraket_Class; } add_action('bp_init', 'Init_Webraket_Class'); ?>
The problem with this is that you're trying to hook a function to wp_ajax_, which itself is hooked to bp_init priority 10, within a class that's not instantiated until bp_init priority 10. While this doesn't preclude success (something I learned from Jeff Sayre's WordPress Hook Sniffer), it's doesn't mean that it will fail more times that not. In the case of the wp_ajax_ hook, the latest you could load your class would be like this
function Init_Webraket_Class(){ $webraket = new Webraket_Class; } add_action('bp_init', 'Init_Webraket_Class', 9);
but even better would be
function Init_Webraket_Class(){ $webraket = new Webraket_Class; } add_action('bp_include', 'Init_Webraket_Class');
or like boone stated on #3622
function Init_Webraket_Class(){ $webraket = new Webraket_Class; } add_action('bp_loaded', 'Init_Webraket_Class');
However when a developer only tests on /plugins and BuddyPress is loaded before the plugin, they won't have problems hooking into init. (until a multisite user drops their plugin into /mu-plugins)
It's true that plugins in the mu-plugins folder using the init hook will find that BP isn't already loaded, but that's the nature of the mu-plugins folder. However, just because a plugin is in the /plugins folder doesn't mean that one will have success. It's a crap shoot, which I've learned from personal experience, and again, from Jeff Sayre's WordPress Hook Sniffer.
@webraket I'll post some example code on #3622 on how to get your code working better.
#10
@
13 years ago
- Milestone changed from 1.6 to Future Release
Groan, this issue is so complicated. No time in this milestone. I hope someone can step in and clarify everything in just a few sentences, or a beautifully simple patch :)
#12
@
8 years ago
- Milestone Future Release deleted
- Resolution set to invalid
- Status changed from new to closed
I am pretty baffled with this ticket. Going to close it and if someone wants to clarify and thinks it's still an issue in modern BuddyPress, please open a new ticket.
I think setting it to 8 does hurt some things, for example the title is set at priority 9.
http://buddypress.trac.wordpress.org/browser/trunk/bp-core/bp-core-hooks.php#L33
My suggestion would still be to never hook core functions on bp_init.
WordPress also never hooks core functions to their init action.
http://codex.wordpress.org/Plugin_API/Action_Reference/init
Even if a plugin hooks to bp_init with priority 1, BuddyPress should be fully initialised at that point.