Changing hooked-to action functions to make new BP hooks work for 3rd-party components

[jeffsayre]

This is an important fix with a long explanation.

Currently, there are five new action hooks in BuddyPress 1.2.x that are intended to be used by 3rd-party components but in reality they do not work. Here's why.

NOTE: Although this explanation focuses on the bp_setup_nav action event (hook), the following logic also applies to these BuddyPress action hooks as well: bp_setup_globals, bp_setup_root_components_action, bp_register_widgets, bp_register_activity_actions.

Suggested patches to all of these hooks are provided in the attached file(s).

---

Here’s the array of all action functions associated with the plugins_loaded action event. The element keys represent the priority set for each added action function. If a given action function does not have a priority set, it automatically is set to a default of “10”.

[plugins_loaded] => Array (

[0] => Array ( [wp_maybe_load_widgets] => Array ( [function] => wp_maybe_load_widgets [accepted_args] => 1 ) [wp_maybe_load_embeds] => Array ( [function] => wp_maybe_load_embeds [accepted_args] => 1 ) )

[2] => Array ( [bp_core_setup_root_uris] => Array ( [function] => bp_core_setup_root_uris [accepted_args] => 1 ) [bp_setup_root_components] => Array ( [function] => bp_setup_root_components [accepted_args] => 1 ) )

[3] => Array ( [bp_core_set_uri_globals] => Array ( [function] => bp_core_set_uri_globals [accepted_args] => 1 ) )

[5] => Array ( [bp_core_load_buddypress_textdomain] => Array ( [function] => bp_core_load_buddypress_textdomain [accepted_args] => 1 ) [bp_setup_globals] => Array ( [function] => bp_setup_globals [accepted_args] => 1 ) )

[10] => Array ( [bp_setup_nav] => Array ( [function] => bp_setup_nav [accepted_args] => 1 ) [bp_setup_widgets] => Array ( [function] => bp_setup_widgets [accepted_args] => 1 ) [bp_register_activity_actions] => Array ( [function] => bp_register_activity_actions [accepted_args] => 1 ) [bp_loaded] => Array ( [function] => bp_loaded [accepted_args] => 1 ) ) ) [shutdown] => Array ( [1] => Array ( [wp_ob_end_flush_all] => Array ( [function] => wp_ob_end_flush_all [accepted_args] => 1 ) )

)

When the do_action function is processing a call from a given action event [ in this case do_action( ‘plugins_loaded’, ‘...’ ) ], it will sequentially loop through this array, firing the first action function in the first array element then proceeding to the next. A given array element can have more than one added action function. This happens when the same priority is set in the add_action call. In fact, you can see that the third array element is the only one that does not have more than one action function.

The reason that the bp_setup_nav hook does not work for any BP-dependent plugins can be discovered by looking at the fifth element in the plugins_loaded array (the one with a key of “10”).

All BuddyPress plugins now have a means of activating only if and when BuddyPress is active by hooking into the bp_init action event. The function that contains that hook is bp_loaded found on line 65 of the trunk version bp-loader.php. This function is hooked to the plugins_loaded event.

Once again, look at the fifth element in the plugins_loaded array. You will find the reference to the added action function bp_loaded. Now, look at where the reference to the added action function bp_setup_nav is located. It is the very first subarray element in the fifth element in the plugins_loaded array.

What does this mean? It means that the bp_setup_nav action function will fire before the bp_loaded action function fires. Since the bp_setup_nav action function contains the bp_setup_nav hook, it means that that hook will be finished firing before the bp_init hook is fired. Which means it gets fired before any BP-dependent plugins have been initialized.

In this case, it is useless for BP-dependent plugins to attempt to hook into the bp_setup_nav action event. By the time they are active, it is already too late.

Possible Fixes:

• Change the priority to 11 or higher
• Hook the bp_setup_nav function to the bp_init action event instead
• Create a separate, new action event exclusively for hooking these five action functions

Although it is possible to fix each of the five BuddyPress action hooks by simply adding a priority lower than 10 to the plugins_loaded action event, I suggest hooking these action functions to the bp_init action event instead. It makes more sense to base the triggering of theses five action events on the bp-load action event. This is what has been done in the patch file(s). However, even with that change, the priority for each added action function will still need to be set to lower than 10 to allow for all dependent plugins to load before the action events are triggered.

[johnjamesjacoby]

Originally those actions hooked into bp_init, but it was causing issues with ajax page loading.

For 1.3 I'll end up taking a better look at these hooks. They were put in last minute (reluctantly because apeatling knew better) and I insisted we at least get them in place early so we can look more into how exactly these should work.

Thanks for doing the dirty work. Very awesome.

[jeffsayre]

JJJ-

Was it interfering with a particular ajax action hook? If so, let me know. I have written what I call a WP Hook Sniffer plugin that ferrets out the firing sequence of all WP / BP hooks. I can determine what's firing when and what needs to be changed to make hooks fire in the proper sequence. I will be making this plugin available soon.

[apeatling]

This is very important, but since no plugins use it, and it's a little too touchy to fiddle with in the branch we should leave it for 1.3. Thanks for the analysis, it'll be super helpful for fixing this up.

[johnjamesjacoby]

Working patch

[jeffsayre]

As mentioned above, if you want help in figuring out any WordPress or BuddyPress hook issue, I have published my WordPress Hook Sniffer plugin available here:  http://wordpress.org/extend/plugins/wordpress-hook-sniffer/

[johnjamesjacoby]

(In [3098]) Fixes #2325 (based on assessment by jeffsayre)

[jeffsayre]

Looks good so far. I still need to run give it a more rigorous testing, running it through my WP Hook Sniffer to look for issues. In particular, I'll be looking at priority settings as I had issues with some of them ibefore. One initial note, I would suggest changing the hooked to actions in these spots as well as these functions are dependent on BP and WP.

bp-core.php:

bp_core_setup_root_uris() --> Line 199 hook to bp_loaded event instead of plugins_loaded
bp_core_load_buddypress_textdomain() --> Line 1956 hook to bp_loaded event instead of plugins_loaded

bp-activity.php:

bp_register_activity_actions() --> Line 1195 hook to bp_loaded event instead of plugins_loaded

bp-core-catchuri.php:

bp_core_set_uri_globals() --> Line 163 hook to bp_loaded event instead of plugins_loaded

[jeffsayre]

Correction to above statement:

these functions are dependent on BP and WP.

Shoule read:

these functions are dependent on BP and not WP.

[johnjamesjacoby]

Jeff,

I made some adjustments to your recommendations. I'd like to keep as many things off of bp_loaded as possible and continue to use bp_init and the rest of the bp-core.php actions wherever possible.

Please SVN up again and test these changes at your earliest convenience. :)

[johnjamesjacoby]

Thinking of reverting these. They work for subdomain, but broke on subdirectory.

[johnjamesjacoby]

Note: Currently futzing with these to try and get them to work in all installs.

[johnjamesjacoby]

(In [3103]) Fixes #2325... again.

[johnjamesjacoby]

Okay... Got this under control and working for all installs (single blog/multi blog) in several environments (windows/linux) with with any front-page variation.

[jeffsayre]

Two issues encountered with this new patch.

The first issue, none of the core activity actions are loaded into the $bp->activity->actions array. This is because action hook bp_register_activity_actions is triggered by the bp_init action hook on line 1195 of bp-activity.php.

The action hook bp_register_activity_actions is used by bp_blogs_register_activity_actions(), friends_register_activity_actions(), groups_register_activity_actions(), and xprofile_register_activity_actions. If you look at these functions, you will see that they each require that an id be set in $bp->xxx->id where the “xxx” is the component name. But, where is $bp->xxx->id set?

It turns out that it is set via individual component action functions tied to the action event bp_setup_globals. See the function bp_blogs_setup_globals on line 65 of bp-blogs.php as an example.

But, if you look at the firing sequence of the action hooks, you will see that the bp_init action hook fires before the bp_setup_globals hook ( See Action_hook_firing_sequence_1.png ). This means that $bp->xxx->id does not get set so the function bp_activity_set_action returns false on line 976 of bp-activity.php. This results in the component’s activity actions not getting registered.

The fix is to hook bp_register_activity_actions to the bp-loaded event, not bp-init. But, it must be hooked to bp-loaded after bp_setup_globals. By making the following change on line 1195 of bp-activity.php that can be accomplished:

Instead of: add_action( 'bp_init', 'bp_register_activity_actions' );

Change it to: add_action( 'bp_loaded', 'bp_register_activity_actions' );

Look at the screenshot Action_hook_firing_sequence_2.png to see how the action hook firing sequence is changed with the above fix.

NOTE: This action hook firing sequence issue was figured out by my WordPress Hook Sniffer Plugin referenced above.

The second issue is that the bp_init event should be triggered after all the core events have been triggered. Why? Because we want to make sure that 3rd-party components hook into BP after all necessary code has been loaded. As it stands now, any component that hooks into BP using bp_init will do so before the bp_setup_globals event is triggered. This means that key globals will not yet be set.

So, change line 2063 of bp-core.php simply by adding a custom priority:
add_action( 'bp_loaded', 'bp_init', 9 );

This will force bp_init to be triggered after bp_setup_globals event.

[johnjamesjacoby]

Give it a check now. I hate doing this kind of testing in the branch and note: this is the last time we're doing it. :)

I added another action: bp_include();

bp_include fires AHEAD of the other core actions, basically in place of where bp_init was before. What I do a lot for clients is only include the BuddyPress files if BuddyPress is actually around, and those inclusions don't actually process any code, they just load up the files and add their hooks in to other places. In the past I was using bp_init for this, but if the intent of bp_init needs to be to wait until the end, then we still need something at the beginning.

[DJPaul]

Can we clarify when bp_init, bp_loaded and bp_include should be used for?

[jeffsayre]

The changes to the action hooks work fine now. The only remaining issue is to answer Paul's question. In short, what hooks should plugin devs use?

With the most recent changes, I found that I had to change to which action I hooked the privacy loader function. It was hooked to bp_init. But now that fires after bp_setup_root_components, bp_setup_globals, and bp_setup_nav--three actions to which I've hooked functions. So, I changed my loader function to be hooked to bp_include, and all works fine.

So, we will have to tell plugin devs that the hook to which they tie their plugin's loader function will depend on whether they need code loaded before bp_setup_root_components, bp_setup_globals, or bp_setup_nav fires. If they do not, then they should hook their loader function to bp_init. Otherwise, they will have to hook their loader to bp_include.

THe information ion this Codex article will need updating:  http://codex.buddypress.org/how-to-guides/make-your-plugin-buddypress-aware-v1-2/

[johnjamesjacoby]

Yep exactly right. That's why I originally kept bp_init loading before the other actions. If you used bp_init as your overall loader, it would never have the chance to hook into the other core actions.

I had to update all my plugins also, but I saw another ticket saying the adverse.

Either way, this is going to be a pain, and plugin authors might need to adapt their plugins this last and final time. I believe we have all the bases covered finally.

To be fair, WordPress doesn't fire its "init" action until the very end of wp-settings.php, so this mirrors that. Plugin authors could hook really early into "bp_loaded" or anywhere on to "bp_include" and be fine going forward if they also need to attach to the other core actions.

I hate zebras.

[jeffsayre]

I like giraffes. Now, try that with jQuery!