Skip to:
Content

BuddyPress.org

Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#4755 closed defect (bug) (fixed)

Address usage of bp_locate_template()

Reported by: r-a-y Owned by: johnjamesjacoby
Milestone: 1.7 Priority: normal
Severity: normal Version: 1.7
Component: Core Keywords:
Cc: rogercoathup

Description (last modified by r-a-y)

Let's try this again.

One-line summary:
bp_locate_template() does not mirror the same set of templates as bp_get_template_part().

Side-effects:
Currently, bp_locate_template() does not check a theme's custom subdirectories ('buddypress', 'community' ) for templates.

This is a problem when theme compat is on as bp-legacy's templates are located in the 'buddypress' subdirectory.

The main issue is bp_locate_template() is used in these two functions:

  • bp_has_custom_signup_page()
  • bp_has_custom_activation_page()

Since bp_locate_template() is used in these functions, when theme compat is on, this leads to the registration / activation templates not being properly detected and hence the registration hijack that BP usually does fails.

Why do bp_locate_template() and bp_get_template_part() reference different templates?
It's due to this:

add_filter( 'bp_locate_template',   'bp_add_template_locations' );
add_filter( 'bp_get_template_part', 'bp_add_template_locations' );

The 'bp_locate_template' filter does not exist and was removed in r6537.

This leads to the registration / activation templates not being detected properly when theme compat is on.

Conclusion
We need a solution so both bp_locate_template() and bp_get_template_part() references the same templates at all times.

Some potential methods include:

  1. Adding back the 'bp_locate_template' filter that was removed in r6537.
  2. Adding the custom subdirectory locations at the template stack level. Attached patch is an attempt at this.

Attachments (1)

template_stack.01.patch (1.5 KB) - added by r-a-y 9 years ago.

Download all attachments as: .zip

Change History (11)

#1 @r-a-y
9 years ago

  • Summary changed from Template stack: Address usage of bp_locate_template() to Address usage of bp_locate_template()

#2 @rogercoathup
9 years ago

  • Cc rogercoathup added

#3 @r-a-y
9 years ago

  • Description modified (diff)

#4 @r-a-y
9 years ago

  • Description modified (diff)
  • Keywords has-patch added

#5 @boonebgorges
9 years ago

template_stack.01.patch seems like it just duplicates the work done by bp_add_template_locations(). If we can fix this problem (which clearly *is* a problem, as you've described it above - thanks for the clarification) without repeating ourselves, we should do so. So your strategy 1 seems better.

Will there be negative consequences to adding the bp_locate_template filter? Looking at the code, I'm guessing that there are two big reasons for removing the filter in r6537:
a) WP's core template loading functions don't have corresponding filters in their template loading functions (side note - it's this missing feature in WP that makes half of the theme compat code necessary. Grr)
b) It's assumed that bp_locate_template() will generally only be called within the context of bp_get_template_part(), and there's already a perfectly good filter in the latter function, so the latter filter seems redundant.

IMO, (a) is a bad reason for us to omit a filter.

As for (b), if we're really concerned about redundancy, then we should remove the filter in bp_get_template_part() and reinstate the filter in bp_locate_template(). Then we'd only have to run bp_add_template_locations() in one place, at the very end of the process.

My vote is to re-add the bp_locate_template filter and call it good enough, unless there's something I'm missing.

#6 @r-a-y
9 years ago

As for (b), if we're really concerned about redundancy, then we should remove the filter in bp_get_template_part() and reinstate the filter in bp_locate_template(). Then we'd only have to run bp_add_template_locations() in one place, at the very end of the process.

I don't mind this approach as we should only run bp_add_template_locations() once.

#7 @johnjamesjacoby
9 years ago

  • Keywords needs-refresh added; dev-feedback has-patch removed
  • Owner set to johnjamesjacoby
  • Status changed from new to assigned

Meee!

#8 @johnjamesjacoby
9 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [6780]) Theme Compatibility:

  • When locating template files, filter 'bbp_get_template_stack' rather than 'bbp_get_template_part'
  • Fixes bug where using bbp_locate_template() directly would result in missing subdirectory locations.
  • Props r-a-y.
  • Fixes #4755.

#9 @grosbouff
9 years ago

Hi, I just updated to rev. 6783, and I still get the bug from ticket #4820 (set as duplicate of this one), which is supposed to have been fixed with this ticket (at rev. 6780). I did a quick test and the same bug still occurs... Is it normal ? Thanks !

#10 @r-a-y
9 years ago

  • Keywords needs-refresh removed

Hi grosbouff,

I'm going to reply at your other ticket (#4820).

Note: See TracTickets for help on using tickets.