[buddypress-trac] [BuddyPress] #4846: Theme compat is overaggressive in loading its files, especially JS
noreply at wordpress.org
Fri Feb 22 03:22:25 UTC 2013
#4846: Theme compat is overaggressive in loading its files, especially JS
Reporter: boonebgorges | Owner:
Type: defect (bug) | Status: new
Priority: high | Milestone: 1.7
Component: Theme | Version: 1.7
Severity: critical | Keywords: 2nd-opinion
(This bug was originally reported to me by imath - thanks!)
A number of popular BuddyPress standalone themes ("standalone" in the
sense that they are *not* bp-default themes) load bp-default's global.js,
either directly from bp-default or from a copy shipped with the theme.
On these sorts of themes, theme compat will kick in - it only bails when
the theme is (a child of) bp-default:
/buddypress-functions.php#L47 This is generally not a huge problem,
because the theme's native templates will be loaded by
`bp_core_load_template()` and the theme compat layer will not kick in. So,
it's not ideal, but it's not harmful either.
However, bp-legacy's JS *is* a problem. When bp-legacy-js is loaded
alongside dtheme-ajax-js, it means that AJAX actions are fired off twice,
which causes lots of problems: activity items are double-posted, activity
deletes fail because the item has already been deleted, etc.
There are a couple of strategies we might employ to fix this.
1. Load bp-legacy-js as late as possible, to give other themes plenty of
time to enqueue. Then, before enqueuing, check in WP's JS enqueue stack to
see if something called dtheme-ajax-js is already enqueued, and bail if
found. This would probably cover most cases, because it looks like most
themes (at least those I've checked) have used 'dtheme-ajax-js' to name
their JS (they just copied it over from bp-default I guess).
2. Do a better job of automatically determining when theme compat should
run. I'm not sure what this would be. We could, for instance, check to see
if a file exists called _inc/global.js. This seems like a pretty lousy
solution in a couple ways, but I'm just throwing it out there.
3. In addition to checking `get_template() == 'bp-default'` and
`get_stylesheet() == 'bp-default'`, we could also do something like:
`current_theme_supports( 'buddypress-ajax' )`. Then, we do a big publicity
push to get theme authors to register their theme as having 'buddypress-
ajax' support if they plan to load their own JS for AJAX. The upside of
this strategy is that it introduces pretty much zero overhead for us, and
it will never result in false negatives (cases where theme compat should
load its JS but doesn't). The downside, of course, is that it requires
action by theme authors.
4. In bp-legacy, do the following:
- First, add prefixes to all of the ajax hooks
/buddypress-functions.php#L137. Eg, 'post_update' becomes
- Then, after bp-legacy has hooked all of its AJAX handlers, loop
through all of the non-prefixed handlers and *unhook* them. That way, the
theme's JS will still load, and AJAX requests will still be sent, but they
won't do anything because WP won't be expecting them.
This fourth strategy is good because it's something that's within our
control, and is pretty much guaranteed to catch all cases of rogue themes.
It's bad because it doesn't prevent the duplicate AJAX requests from
firing, which results in additional overhead (as multiple instances of WP
will spin up each time a request is sent). Of course, this latter issue
would clear itself up as theme authors fixed their themes.
I personally lean toward a combination of 3 (best practice) and 4
(failsafe), but it's possible that I'm missing something. In any case, we
really have to do *something*, because as it currently stands, these
themes are largely broken on 1.7.
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/4846>
More information about the buddypress-trac