add_theme_support( 'buddypress' ) should not load buddypress-functions.php

[modemlooper]

bp-default derived themes copied over functions.php but theme compat uses buddypress-functions.php. If you use add_theme_support( 'buddypress' ) it loads buddypress-function.php and your functions file which can cause duplicate buttons, duplicate secondary avatars and any other code copied from bp-default.

[boonebgorges]

This shouldn't happen. It's true that we load buddypress-functions.php, but none of the items (avatars, buttons, etc) should load, because of the check that we do in the constructor: ​https://buddypress.trac.wordpress.org/browser/trunk/bp-templates/bp-legacy/buddypress-functions.php#L47

Can you please share your child theme, or the relevant parts of it, so that the devs can attempt to reproduce the problem?

[modemlooper]

Actually, it may be because I'm using a theme switch for mobile.

I have twenty twelve active on desktop so it's using theme compat but the switched theme is a derivative of bp-default. I can change functions.php into buddypress-functions.php for a fix but I think add_theme_support( 'buddypress' ) should not load buddypress-functions.php

[boonebgorges]

I think add_theme_support( 'buddypress' ) should not load buddypress-functions.php

To repeat: while it's true that we are loading the file, it shouldn't actually do anything as long if your current_theme_supports( 'buddypress' ). So I don't see a functional difference between what we currently do and what you are suggesting.

I'm guessing that you have copied buddypress-functions.php into your own theme, and in the process you are double-defining many function names. Didn't you have to change them to clear duplicate function name errors? Again, if you would be willing to share the code that you are using, it would be a lot easier to help to debug the issue.

[modemlooper]

I'm not using any code. It's a theme inside wp-content/themes that was a derivative of bp-default. Theme switcher on mobile switches theme but since the desktop is using theme compat BuddyPress is loading buddypress-functions.php but the theme that is the derivative includes functions.php so it's duplicating some functions that add buttons and secondary images. I placed add_theme_support( 'buddypress' ) in the bp-default derived theme and it should block buddypress-functions.php

I could remove the extra functions but this won't fix the issue of loading both files.

[boonebgorges]

Is "theme switcher" a plugin? Would you mind sending a link so that I can see how it accomplishes this on-the-fly switch?

[modemlooper]

no plugin. my code.

	function get_stylesheet( $stylesheet ) {

		if ( wp_is_mobile() ) {
			return 'child';
		} else {
			return $stylesheet;
		}
	}

	function get_template( $template ) {
		if ( wp_is_mobile() ) {
			return 'child';
		} else {
			return $template;
		}
	}

	add_filter( 'template', 'get_template' );
	add_filter( 'stylesheet',  'get_stylesheet' );

[modemlooper]

I should add, this is an issue when you are using a full theme and not a child of bp-default and you include a copied functions.php from bp-default

[johnjamesjacoby]

If it's happening, I'm okay with this for now. It shouldn't be happening though, since BP_Theme_Compat::start() bails if the current theme supports BuddyPress. It's possible there's a load-order issue here, but that said, if you need to, you can completely unhook bp_load_theme_functions() for now if it interferes with anything.

remove_action( 'bp_after_setup_theme', 'bp_load_theme_functions', 1 );

[modemlooper]

That works!

[boonebgorges]

I've seen this issue come up in other contexts, so I'm moving to 1.9 to try to sort it out.

[hnla]

I opened a ticket on the add_theme_support issue @ #5142. As JJJ surmises it looks essentially to be a load order issue, if add_theme_support('buddypress') is correctly hooked on 'after_setup_theme' then class BP_Theme_compat will not bail as it ? runs too early to read add_theme_support - move add_theme_support out of any hook and the BP class now gets the value and does bail.

[r-a-y]

As hnla alluded to in #5142, this is definitely a load order issue.

BP_Theme_Compat::start() runs after the buddypress-functions.php file is loaded and that functions file contains the extended BP_Theme_Compat class. Chicken before egg!

01.patch moves the enable theme compat logic into bp_load_theme_functions() so we can determine whether we should load up buddypress-functions.php.

I've also introduced new functions - bp_enable_theme_compat() and bp_set_enable_theme_compat().

bp_enable_theme_compat() is extremely useful for plugin devs as it helps in determining whether a plugin should serve up templates depending if theme compat or bp-default is enabled.

The only thing I'm worried about in the patch is deregistering the default theme compat template stack. Could use some dev feedback on this.

[boonebgorges]

Good find, r-a-y.

The only thing I'm worried about in the patch is deregistering the default theme compat template stack. Could use some dev feedback on this.

I'm not sure the point of doing this. If theme compat is disabled, what's the difference if the template stack is registered?

Function naming is non-ideal, IMO. bp_enable_theme_compat() sounds like a verb ("hey BP, enable theme compat!") but it's actually a check. Maybe better: bp_is_theme_compat_enabled()? The other function, bp_set_enable_theme_compat(), also makes it sound as if theme compat is being enabled, when really it's *detecting whether* it should be enabled. Maybe better: bp_set_theme_compat()?

[r-a-y]

If theme compat is disabled, what's the difference if the template stack is registered?

I believe I remember reading in another ticket that it might be advantageous for bp-legacy templates to "leak" through. If you remove the bp_deregister_template_stack() line, bp-legacy templates will indeed be used as a fallback if the templates don't exist in the parent / child theme. I think we should deregister it as it makes logistical sense. Just wanted a second opinion!

(As I was looking for this ticket, Boone actually mentioned the bp_load_theme_functions() approach in ticket:4846#comment:4.)

I'm fine with your function naming, Boone! Was kind of contemplating the same thing as I was writing the code.

[boonebgorges]

I played with this a bit more, and I think that even my proposed function names are not clear enough. The issue is that we already have functions called is_theme_compat_active(); the difference between that and _enabled() is far from clear. What the new functions do is to detect whether the *current theme needs theme compat*. So I've renamed them accordingly. Let me know what you think about this.

I believe I remember reading in another ticket that it might be advantageous for bp-legacy templates to "leak" through. If you remove the bp_deregister_template_stack() line, bp-legacy templates will indeed be used as a fallback if the templates don't exist in the parent / child theme. I think we should deregister it as it makes logistical sense. Just wanted a second opinion!

Deregistering the template pack seems like a bit of a roundabout way to achieve this end - it's not very transparent, and would be pretty tricky to debug if you didn't already know what was up. A simple procedural check seems better to me, so we need a single point where we can check to see whether theme compat is required for the given theme, and bail if necessary. I've chosen the beginning of bp_theme_compat_reset_post(), because it's a single point that is called throughout the BP_Theme_Compat children in the individual components; if the dummy post doesn't get reset in this function, theme compat will fail. This seems a bit more elegant to me, but I'm open to feedback.

[r-a-y]

Nice changes in the function naming, Boone!

I think hooking into bp_theme_compat_reset_post() might even be too late. Doing the check there assumes that this function is called and hooked on the 'bp_template_include_reset_dummy_post_data' action.

Might be better to add the bp_use_theme_compat_with_current_theme() check at the beginning of bp_template_include_theme_compat().

[boonebgorges]

In 7432:

Do a better job preventing theme compat from running when current_theme_supports( 'buddypress' )

This changeset ensures that when a theme declares add_theme_support( 'buddypress' ),
BuddyPress will not load any of its theme compatibility layer. Previously,
'buddypress' support would prevent templates from loading, but would still
allow the buddypress-functions.php file to load in certain cases.

To this end, two new functions are introduced:

• bp_detect_theme_compat_with_current_theme(), which centralizes the logic used by BuddyPress to detect whether the current theme will require theme compatibility (themes do not need theme compat when current_theme_supports( 'buddypress' ), when it's related to bp-default, or when members/members-loop.php is found). The function sets a global flag which can be checked later.
• bp_use_theme_compat_with_current_theme(), which is used to check the flag set in bp_detect_theme_compat_with_current_theme().

Fixes #4879

Props r-a-y