Templates: locating empty array of template(s) errors undefined variable

[Offereins]

In order to prevent loading a specific template part, in my case the 'activity/post-form' (a brilliant thought of mine to make the activity stream contain only actual site activity), I empty the array of templates to locate through filtering bp_get_template_part.

Thus follows sending an empty array into bp_locate_template(). However, here the array will be looped, creating new variables in the loop that do not exist outside of it. Beyond the loop these variables ($template_name being the first) are then used, but since the loop was never run, the variable appears to be undefined.

What I expect is that the template loading dies silently when there appear to be no templates requested.

To fix this, I suggest to only run bp_locate_template() when the list of templates is non-empty. Patch is attached.

[DJPaul]

Easily re-produced:

wp> bp_locate_template(array())
PHP Notice: Undefined variable: template_name in /srv/www/buddypress.dev/src/wp-content/plugins/buddypress/src/bp-core/bp-core-template-loader.php on line 138

@Offereins Thanks for the report and patch. How do you feel about making this function return an empty string if a template can't be found? If so, let's put a since tag on the PHPDoc explaining the return type change.

I know it returns a bool already, but I think that's just bad function design. An empty string is a false-y value, so it'd probably won't break anything unless someone's doing a type-strict check on the output of bp_locate_template. We/you would need to check the instances of this function in BuddyPress core to check if this breaks anything.

[Offereins]

@DJPaul Thanks for confirming the bug. In reply to your suggestion: returning a false-y string is okay with me. Most return value evaluations of the function are loosely compared, but I've found the following strict-type checks for output of bp_locate_template():

• bp-core/bp-core-template-loader.php @ line 175 in bp_locate_template_asset()
• bp-groups/classes/class-bp-group-extension.php @ line 974 in BP_Group_Extension::setup_edit_hooks() (compares empty string)

Furthermore, bp_locate_template() constructs the return value for:

• bp_get_template_part() of which no value evaluations are found, but is widely used
• bp_displayed_user_get_front_template() which has a strict type check in:
  • bp-members/bp-members-template.php @ line 1379 in bp_displayed_user_front_template_part()
• bp_groups_get_front_template() which has a strict type check in:
  • bp-groups/bp-groups-template.php @ line 4189 in bp_groups_front_template_part()

My conclusion is that this propagates to somewhat more functionality than you'd originally guess.

[mercime]

Sorry, have to move this to future release.

[DJPaul]

OK, let's just get this patch in for 2.8, @Offereins. We'll need to add the { } brackets, but that's just a minor tweak.

[Offereins]

Please ignore 7149.01.patch

[Offereins]

Return false, not an empty string

[Offereins]

@DJPaul See attached, 7149.03.patch is tha one.

[DJPaul]

This looks good, @Offereins! I'll let you commit it once your access comes through.

[offereins]

In 11267:

Stop locating an empty set of templates.

Bail early in bp_locate_template() when there are no files to look for.
This prevents PHP from showing errors when an empty array is provided. Use
case for this is when by hooking into the bp_get_template_part filter
you'd like to prevent the locating of a template.

Fixes #7149.