[buddypress-trac] [BuddyPress] #3985: Race condition processing Ajax requests.
buddypress-trac at lists.automattic.com
buddypress-trac at lists.automattic.com
Wed Apr 18 16:33:32 UTC 2012
#3985: Race condition processing Ajax requests.
----------------------------------------+-----------------------
Reporter: gary_mazz | Owner: DJPaul
Type: defect (bug) | Status: assigned
Priority: normal | Milestone: 1.6
Component: Core | Version: 1.5.3
Severity: major | Resolution:
Keywords: dev-feedback needs-testing |
----------------------------------------+-----------------------
Comment (by boonebgorges):
Thanks for all your work on this, Paul. Looks pretty good, with one
problem.
I've applied the patch against the current trunk, with the following theme
setups:
1. BP-Default
2. Child theme of BP-Default, which allows bp-default to use its native
ajax.php and global.js (Status)
3. A theme adapted for BP using a recent version of the BP Template Pack
(loads ajax.php and global.js directly from bp-default, so this is
essentially the same as 2)
4. A theme with manually-added BP compatibility: global.js and ajax.php
(old versions) copied into the theme, and enqueued/included manually
As expected, 1-3 all work correctly. Since they all use ajax.php directly
from bp-default, they all get the improvements automatically.
In case (4), I'm getting stray '0's. (When clicking Favorite in the
activity stream, for instance.) Basically, the problem that Paul outlines
above. I have a feeling that this is going to affect a pretty fair number
of folks (especially people who have done heavy customization, who are
probably some of our eagerest users), so we should come up with some way
to help them adjust. I have a couple of ideas:
- Publicize the change heavily. Tell theme devs: Either you've got to copy
over your included ajax.php with the new one, *or* you have to manually
merge the changes in (if you have customized the file heavily, this might
be the better option). Or, we could tell them to unhook our new admin-
ajax.php method and manually provide their own `ajaxurl`, like we always
have (they could fire up the deprecated function, maybe). (This latter
idea seems bad for a number of reasons, but I'm just throwing it out
there.)
- Hack together a fix for these users. Since the problem is that the
handler is not killing PHP execution before hitting the end of admin-
ajax.php, we could rig BP to do it on behalf of the offending theme. The
only place where we can really do it is on the `wp_ajax_` and
`wp_ajax_nopriv_` hooks. Something like this, in one of our core files
(*not* the theme):
{{{
// Only load when we detect that the old ajax.php is present
if ( !function_exists( 'bp_dtheme_register_actions' ) ) :
function bp_kill_ajax_output() {
$actions = array(
// List all the offending callback functions
'activity_mark_fav' =>
'bp_dtheme_mark_activity_favorite',
'activity_mark_unfav' =>
'bp_dtheme_unmark_activity_favorite'
// etc
);
foreach( $actions as $name => $function ) {
add_action( 'wp_ajax_' . $name,
create_function( '', 'die();' ), 9999 );
add_action( 'wp_ajax_nopriv_' . $name,
create_function( '', 'die();' ), 9999 );
}
}
add_action( 'after_setup_theme', 'bp_kill_ajax_output', 20 );
endif;
}}}
This is an ugly and pretty hackish solution. But: it fixes the '0' problem
on all setups, for more graceful degradation.
My feeling about this is that we should be as generous as possible to
theme developers, who really got jerked around when 1.5 dropped. IMO we
should do *both* of these routes (education as well as a fallback method).
--
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/3985#comment:14>
BuddyPress <http://buddypress.org/>
BuddyPress
More information about the buddypress-trac
mailing list