[buddypress-trac] [BuddyPress] #4846: Theme compat is overaggressive in loading its files, especially JS

[buddypress-trac]

#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:
 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.

 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
 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.

-- 
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/4846>
BuddyPress <http://buddypress.org/>
BuddyPress