Theme compat is overaggressive in loading its files, especially JS
|Reported by:||boonebgorges||Owned by:|
(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: https://buddypress.trac.wordpress.org/browser/trunk/bp-templates/bp-legacy/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.
- 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).
- 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.
- 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.
- In bp-legacy, do the following:
- First, add prefixes to all of the ajax hooks https://buddypress.trac.wordpress.org/browser/trunk/bp-templates/bp-legacy/buddypress-functions.php#L137. Eg, 'post_update' becomes 'bp_legacy_post_update'.
- 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.