Remove HTML from post_title for groups

[alexander.rohmann]

Hi there,

This is painful theme compatibility issue. Why is HTML being placed on the post object? This breaks theme headers that use
the_title or get_the_title in places like meta tags, or anywhere else a link might not be desired.

public function single_dummy_post() {
		bp_theme_compat_reset_post( array(
			'ID'             => 0,
			'post_title'     => '<a href="' . bp_get_group_permalink( groups_get_current_group() ) . '">' . bp_get_current_group_name() . '</a>',
			'post_author'    => 0,
			'post_date'      => 0,
			'post_content'   => '',
			'post_type'      => 'page',
			'post_status'    => 'publish',
			'is_page'        => true,
			'comment_status' => 'closed'
		) );
	}

​https://buddypress.trac.wordpress.org/browser/trunk/src/bp-groups/bp-groups-screens.php#L1225

Surely this can be refactored.

Thanks!

[hnla]

Think the question is better put as "Why do we feel the need to create a link for the group name on the groups single screens" it's a bit like a single WP post having it's post title linked to itself, it doesn't only on the loops lists would it be. Was wondering about this the other day!

[alexander.rohmann]

Replying to hnla:

Think the question is better put as "Why do we feel the need to create a link for the group name on the groups single screens" it's a bit like a single WP post having it's post title linked to itself, it doesn't only on the loops lists would it be. Was wondering about this the other day!

Yeah, exactly. It's only being added in single view. If that link is absolutely needed, it could just be added in the template files.

Doing this is working fine for me:

'post_title' => bp_get_current_group_name(),

Any objections to changing it to simply that? Or am I missing something perhaps?

[imath]

Replying to alexander.rohmann:

Yeah, exactly. It's only being added in single view. If that link is absolutely needed, it could just be added in the template files.

As this is outputted by the <?php the_title() ;?> tag of the page.php template for instance, except from adding another title (and as a result have 2 titles for the page), i don't think it's a good idea to manage it from the groups-header template.

I think the link is interesting to come back to group's home, when you are on one of the "sub" page of the group, just like you can always have a link to your site's home page from the title of the website on any page of the site.

If i was designing a theme, i think i'd use single_post_title() instead of the_title() when adding the post title to a meta tag or more globally wp_title() with custom parameters.

But that is my humble opinion about it.

[hnla]

I think the link is interesting to come back to group's home, when you are on one of the "sub" page of the group, just like you can always have a link to your site's home page from the title of the website on any page of the site.

hmm fair enough, not sure though it's that necessary, is it that hard to navigate back to the group home? - Point taken about the_title() and output in page.php though that unless ones running custom template overrides is tricky.

[alexander.rohmann]

Forcing the property is very invasive for something as trivial as having the convenience to get back to the group home page.

Furthermore, when I think that any developer who asks for the_title should be able to expect simply that - not that plus something else. It's just an anti-pattern. It might be a few more lines of code in the template files, but at least everything is explicit and there's no "magic" that leaks into unforeseen places.

I feel as if we should be able to expect the post_title property to behave as if it had been validated and sanitized the same way any other post has been. WordPress doesn't allow HTML in post titles, so this feels like more of a hack than a proper solution.

Using single_post_title doesn't give you the specific group name. wp_title() is an option, but what if I just want the exact title of the current page? Say I'm adding meta like og:title, It

Another example is how theme commonly use get_the_title() to create breadcrumb navigation. Breadcrumbs also supply links in their own format. I don't want to add a condition to check if I'm on the group page just so I know that I already have the link in place. What if I want breadcrumbs without a link at all?

[hnla]

It might be a few more lines of code in the template files,

Problem is that BP's template hierarchy in theme compat uses page.php from what ever theme is activated as the lowest common denominator so we can't hack that to add link markup.

imho we should remove the link, however then we need to consider backpat but surely we could filter the link back in so it could be returned false if not required.

[imath]

mm, what about private or password protected posts or pages.. Are we getting only the title in these cases ?

The only thing that comes to my mind is to allow you to easily remove the link. This leaves the choice to others to keep it.
See 6008.patch

[alexander.rohmann]

I don't think private/protected posts really matter. I was just hoping for the property to be consistent. That filter would be great!

[johnjamesjacoby]

Let's remove the link; I think it causes more unintended harm than intended good. There are a few other places where we do this, so worth looking at other reset_post usages.

[DJPaul]

I guess changing this one might be OK (it highlights how much we need better breadcrumb-ing in our templates) but while it would fix the problem that the reporter is having, it won't fix the issue fully across BuddyPress, so I'm not sure it's worth fixing by itself.

"Create a Group" on the Groups Directory title, for example -- we could remove that, sure, but where do we put the link after it's gone? What backwards compatibility (with bp-default or customised theme compat. templates) concerns would this change introduce?

[boonebgorges]

Why are we not just filtering 'the_title'? It is still somewhat rude to other plugins etc that may be filtering this value, but at least it's not messing with other plugins that may be calling the_title() (as long as we're selective about how we do the filtering).

[johnjamesjacoby]

the_title and the_content are functions that shouldn't really be used creatively. They represent the title and the content of the current post, not a breadcrumb or an HTML attribute or something else (isn't there an attribute specific title function that strips tags anyways?)

I have a hunch the consensus among the leads will be to do whatever breaks the fewest things, while not compromising the best experience.

[imath]

Replying to boonebgorges:

Why are we not just filtering 'the_title'?

The problem : what is the best moment to filter it ? For a bbPress plugin (support topic) i've played in this area and it's quite complex.

get_the_title() (the function where the_title filter is) can be used :

• in meta tags (eg: the facebook og:title meta as explained alexander.rohmann in a previous comment)
• in breadcrumbs
• in widgets
• in footer

When are we absolutely sure to be in the targetted entry title ? I'd say never. There's no hooks before or after that is precise enough from my point of view.

You can wait wp_head, but then sidebar ? Sidebar can be in the header template of the theme (eg: twentyfifteen), or after the_content? So you need to ​toggle a global to true/false on the ​hooks dynamic_sidebar_before / dynamic_sidebar_after

And if the theme has breadcrumbs... Then you need to leave a way for the user to filter your ​filter

So IMHO, the best choices are : remove links in title as johnjamesjacoby suggested or add a filter to leave themes/plugins remove the link when they need > eg ​6008.patch or do nothing : they just could do a strip_tags( get_the_title() ) to absolutely be sure to only get the title for their metas/breadcrumbs ....

[boonebgorges]

Thanks, imath. I appreciate your point, but I still think that we'd be better off with a simple filter than with what we have now. If our filters looked like this:

function bp_groups_directory_page_title_filter( $title ) {
    if ( ! bp_is_directory() || ! bp_is_groups_component() ) {
        return $title;
    }

    return __( 'Groups Directory', 'buddypress' ) . '<a href="whatever">' . __( 'Create a group', 'buddypress' ) . '</a>';
}
add_filter( 'the_title', 'bp_groups_directory_page_title_filter' );

it would have the same effect as what we're doing now, but at least plugins would be able to use remove_filter() to remove our mods if they want to use the_title() for other stuff on the page.

That being said, if others think that removing all HTML from the page title is the best way to go, I won't argue - I agree that it's the best choice in many ways, but we do have to figure out how to deal with concerns like ​DJPaul's. IMHO, adding yet another filter as in 6008.patch​ is no better than the filter strategy I suggest above - theme authors will have to remove_filter() in one case, add_filter() in another.

[imath]

Replying to boonebgorges:

I still think that we'd be better off with a simple filter than with what we have now.

Using the filter you shared in your comment would result in having all titles of an eventual recent posts widgets set to 'Groups Directory' when on the groups directory page. If the post type was specific to the groups directory you could check it but it's a 'page' post type so even if you do a check on the page post type from your filter a "wp_list_page" widget would have all its title set to 'Groups Directory'.

This also means that every time get_the_title() will be used, the filter will be called. That's the main difference with adding a filter just before resetting the post.

So i still think, filtering the_title is not a so great idea.

[boonebgorges]

Using the filter you shared in your comment would result in having all titles of an eventual recent posts widgets set to 'Groups Directory' when on the groups directory page. If the post type was specific to the groups directory you could check it but it's a 'page' post type so even if you do a check on the page post type from your filter a "wp_list_page" widget would have all its title set to 'Groups Directory'.

Sorry, I was writing this off the top of my head. It looks like 'the_title' also receives the post id. We could use that for a reliable check (against the value in bp_pages).

[imath]

Yes, sorry too, i thought of it after my comment :)

But i think the ID should be 0 as we resetting it in the screens file, so it might not be the one of the bp_pages.

I'd also suggest to reference the filter(s) once we are sure it's is_buddypress() territory and very late in do_action( 'wp_head' ). I think we could also take benefits of the various actions templates are containing to remove the filter asap (eg: 'bp_before_directory_groups_page').

[boonebgorges]

Replying to imath:

Yes, sorry too, i thought of it after my comment :)

But i think the ID should be 0 as we resetting it in the screens file, so it might not be the one of the bp_pages.

I'd also suggest to reference the filter(s) once we are sure it's is_buddypress() territory and very late in do_action( 'wp_head' ). I think we could also take benefits of the various actions templates are containing to remove the filter asap (eg: 'bp_before_directory_groups_page').

Gah, I think you're right about 0.

In terms of juggling and carefully timing filters, I think we are already doing enough confusing stuff here. The original problem in this ticket is that we're engaging in some sorcery that effectively cannot be undone by theme developers. My suggestion was to use a simple add_filter(), so that theme devs could use a simple remove_filter() to undo it. If we're going to have to do all sorts of nested add_action() calls, it negates most of the benefit of moving to a filter.

On that note, I guess I lean toward removing the HTML altogether. Let themes decide whether they want the item title to be a link. This will change the existing behavior, but I wonder if anyone will notice (who is clicking on the group title when already looking at the group page?). As for Create a Blog and Create a Group buttons, we'd have to find a new place for them, within 'the_content'. I don't think this by itself is the end of the world, though it's probably worth reading over #5144 [7942] before going too far down this road.

[hnla]

I was about to drop a comment urging everyone to look at this series of comments here and the complexity seemingly that has arisen in working out a filter path as solution but Boone covered it in comment above.

Logically the proper and correct approach now is simply get rid of the markup, it wasn't just that it was hard/impossible for theme developers web developers / site developers to remove but that this function we've hijacked isn't meant to be re-factored with markup it's usually already been prepared and sanitized to deliver a simple string.

but I wonder if anyone will notice (who is clicking on the group title when already looking at the group page?).

Well exactly - if they do and post to the forums I'll produce a template they can copy to replicate the functionality.

On a sidenote am I going mad I thought we addressed the question of the group create button a couple of versions back, how is that it's still being pumped out into the heading tag of templates?
Refreshes his memory on the subject then spots Boon's links to ticket/revision.

[imath]

Replying to hnla:

On a sidenote am I going mad I thought we addressed the question of the group create button a couple of versions back, how is that it's still being pumped out into the heading tag of templates?

As boonebgorges mentionned: in r7942, these two filters are creating the button in the_title() :

add_filter( 'bp_groups_directory_title',   'bp_legacy_theme_group_create_button' ); 
add_filter( 'bp_blogs_directory_title',    'bp_legacy_theme_blog_create_button' );

[hnla]

Replying to imath:

Replying to hnla:

On a sidenote am I going mad I thought we addressed the question of the group create button a couple of versions back, how is that it's still being pumped out into the heading tag of templates?

As boonebgorges mentionned: in r7942, these two filters are creating the button in the_title()

@imath Yes thanks, my memories so terrible, takes a while to remember what was what, made worse by fact I helped on that ticket :)

[imath]

Replying to boonebgorges:

On that note, I guess I lean toward removing the HTML altogether. Let themes decide whether they want the item title

As for Create a Blog and Create a Group buttons, we'd have to find a new place for them, within 'the_content'.

6008.02.patch is my suggestion to remove all html from page titles. About Create a blog / Create a group i think an interesting place can be a new item tab in Sites/Groups directory.

For the Groups directory the result is
All Groups | My Groups | Create a Group

For the Blogs directory the result is
All Sites | My Sites | Create a Site

I'm not sure the 2 functions i've added in case a specific theme is still filtering the blogs/groups directory titles are necessary :

• bp_groups_backcompat_create_group()
• bp_blogs_backcompat_create_blog()

PS @hnla: no problem, it happens to me too :)

[boonebgorges]

Thanks, imath. I think this general approach is, on balance, the best way forward. I would like for the other leads to weigh in, so let's make a note to talk about it during the next dev chat.

Regarding specifics, a couple brief thoughts:

• It would probably be better to make sure that bp_legacy_theme_group_create_button() etc continue to echo the same value as before. Changing the echoed value so that it's wrapped in an <li> will wreak havoc for others using the button. Maybe a different function name altogether?
• How about making this change directly in the template instead of using the bp_groups_directory_group_filter hook? What are the considerations on each side?
• You changed the section // Title based on ability to create blogs but I don't get the change - why would the page title ever be 'Create a Site' on the Sites directory?

[imath]

Replying to boonebgorges:

Thanks a lot for your feedback :)

I would like for the other leads to weigh in, so let's make a note to talk about it during the next dev chat.

Yes, i agree.

• It would probably be better to make sure that bp_legacy_theme_group_create_button() etc continue to echo the same value as before. Changing the echoed value so that it's wrapped in an <li> will wreak havoc for others using the button. Maybe a different function name altogether?

Ok, i guess it's something i haven't thought of. You mean that even if we would no more add the link in the title, someone could have created a specific template for the site's directory with let's say :

<h1><?php echo apply_filters( 'bp_blogs_directory_header', __( 'Sites', 'buddypress' ) );?></h1>

or

<h1><?php echo bp_legacy_theme_blog_create_button( __( 'Sites', 'buddypress' )  );?></h1>

I only thought of themes that could have created a template pack using the filters :

add_filter( 'bp_groups_directory_header',  'bp_specific_theme_group_create_button' );
add_filter( 'bp_blogs_directory_header',   'bp_specific_theme_blog_create_button' );

Because in this case the two functions bp_groups_backcompat_create_group() and bp_blogs_backcompat_create_blog() would move the 'create' button in the item navs

• How about making this change directly in the template instead of using the bp_groups_directory_group_filter hook? What are the considerations on each side?

Why not. But then the two functions i was talking about would not be necessary anymore i think. So we would need to be sure no template packs are using the filters, because it would not be possible to do backcompat i think.
I also thought at the eventuality someone created a specific template in his theme with the 2.1 markup. In this case, if we directly insert the change in the template, i think the user won't have any link to create the group or blog as soon as the user upgrade to 2.2.

• You changed the section // Title based on ability to create blogs but I don't get the change - why would the page title ever be 'Create a Site' on the Sites directory?

Yes i was surprised too at the beginning :) But It's the page used to display the form to create the new site. So it's not the directory dummy post but the BP_Blogs_Theme_Compat->create_dummy_post() one.

[imath]

As discussed in latest dev-chat, here's a screenshot of the effects of the 6008.02.patch (moving create links for groups component and blogs component into the directory nav items)

And as johnjamesjacoby recommended ​here, if no objections arise in the next few hours, i will commit to remove all links from page titles *. We will then have to find the best place to move them before the end of 2.2 dev cycle.

#4638 is a relative.

* As some unit tests are failing i'll commit when tests will be ok.

[imath]

In 9157:

Remove all links in titles of directory and single item pages

Since BuddyPress 1.7, to display the "Create" buttons (Blogs & Groups component) and links to the single item landing pages at a similar place to BP-Default, BP Theme Compat was adding links into the title of the WordPress post object. For 2.2, team has decided to remove all these links and find a better placement for them before the end of the dev-cycle.

See #6008

[imath]

As discussed yesterday in dev-chat, and unless someone else has a better idea before the end of 2.2 dev-cycle, 6008.03.patch is moving the create links into the Groups / Blogs Directory item navs (at the last position). (as illustrated in the screenshot added to ​comment 27).

to follow boonebgorges's recommandation:

It would probably be better to make sure that bp_legacy_theme_group_create_button() etc continue to echo the same value as before. Changing the echoed value so that it's wrapped in an <li> will wreak havoc for others using the button. Maybe a different function name altogether?

I've left unchanged the bp_legacy_theme_[group/blog]_create_button() and created new ones for the nav items.

It's possible to avoid the "create nav items" display using :

add_filter( 'bp_get_group_create_nav_item', '__return_false' );
add_filter( 'bp_get_blog_create_nav_item', '__return_false' );

[DJPaul]

I am against putting an action button or link (semantics seems irrelevant here) inside the "All Groups" / "My Groups" bar because, in the default template pack, those tabs cause the main content of the screen to re-draw, and mixing those things (essentially, filtering the current screen vs linking to another page) will be confusing and it will make the interface harder to understand.

The proposal could be mitigated, for example, if the "Create Group" LI was right-aligned, so that it appeared above the "Order By" dropdown, and if it retained its existing button styling instead of it being a simple text link.

Alternatively, if we think most sites will have too many tabs already, or have made design decision assuming that that UL will only ever filter the main content, another suggestion is to put the button in the same UL as the "Order By" button: ​https://i.imgur.com/TV0rgIl.png

I am much, much happier with the idea of making a change to these templates (and potentially causing the release to make it appear that we've removed the "Create Group" button for certain themes), rather than adding it directly into the main navigation/filtering tabs.

[alexander.rohmann]

Thanks guys! This is great.

My 2 cents would be changing the templates. Even though this suggests changing things for themes, I feel like most sensible theme developers will appreciate the added flexibility at the cost of a little refactoring.

Not to muddy the waters here, but this same issue exists here:

​https://buddypress.trac.wordpress.org/browser/trunk/src/bp-forums/bp-forums-screens.php#L204

I'm not sure if they should be solved in the same manner or not though.

[boonebgorges]

I am much, much happier with the idea of making a change to these templates (and potentially causing the release to make it appear that we've removed the "Create Group" button for certain themes), rather than adding it directly into the main navigation/filtering tabs.

I don't understand the vehement opposition to putting the New link in this place - I have a feeling it's heavily tinted by years of using BP, and internalizing its metaphors, rather than approaching this as a regular user - but I also don't care enough to belabor the point any further. If we're going to move forward with a different solution, we'll need a patch or a mockup.

[hnla]

@DJPaul I'm struggling to see why the objection to the proposed position in the list-tabs, I don't see why 'mixing' these things is confusing to whom? screen readers? they will read a list of items they will read 'link: all groups', 'link: my groups', 'link: create a group' there isn't really anything confusing there, from a visual aspect, equally we see quite a clear indication of what these items do and don't see that that is confusing for users or harder to understand.

As for visual layouts we have such limited options here and frankly it doesn't matter what option we settle on it will upset one theme or another, don't think we can avoid that or overly worry other than hopefully finding the option that has the least impact or disruption.

I'm not clear on what you mean in the last paragraph - remove the button but leave floating free as a new element positioned in the template? It's an option and partly the reason I wanted to add the template update section to the codex so we could offer detailed instruction on changes on a release basis and what needs doing to make adjustments, so I could live with that option and firefight the potential moans :)

[imath]

Replying to DJPaul:

First, i'm not necessarly a huge fan of the positionning i've suggested, it seems to me that it's the one that have the less impact on other parts of the template. Reason why i've took this road.

Alternatively, if we think most sites will have too many tabs already, or have made design decision assuming that that UL will only ever filter the main content, another suggestion is to put the button in the same UL as the "Order By" button: ​https://i.imgur.com/TV0rgIl.png

I think, it's not a better idea, as this part is the subnav of Groups / My Groups, and it seems to me Create a group is not a sub action of Groups or My Groups. Here's an example in 2015 :

The proposal could be mitigated, for example, if the "Create Group" LI was right-aligned, so that it appeared above the "Order By" dropdown, and if it retained its existing button styling instead of it being a simple text link.

I'd say why not, but when i went this road before submitting the first patch, i had a feeling the right part of the template was a bit "overloaded", reason why i thought it wasn't great. Here's an example in 2015 :

Now, if we are to take this road, i'd suggest to also move the search part in order to give some "air" to the right part of the template.
Here's an example in 2015 :

It's better in 2012 :)

Pros:

• the search can be a sub action of Groups or My groups
• it avoids the negative margin applied to #buddypress div.dir-search which is really not great in 2015.
• we are already doing this for the single group members template, here's a screenshot of it:

Cons:

• we need to also change the search positionning for the members directory, and the activity directory
• At this place in the activity directory, we already have the RSS link :(

Honestly, like boonebgorges i don't really care about the positionning of the button as soon as we try to make everything we can so that once the users will have upgraded to 2.2, they will be able to find all pieces into their template.

IMHO, the positionning in 6008.03.patch is not the best, but is the least-worst :)

[DJPaul]

Let's go with 6008.03.patch for 2.2.

[imath]

Replying to DJPaul:

Let's go with 6008.03.patch for 2.2.

Thanks for your comment DJPaul, i'm going to commit the patch and leave the ticket open.

[imath]

In 9291:

Move the Blogs and Groups create links into their directory main navs

Since r9157, we have removed the create links of these two components from the title of their WordPress post object.
For version 2.2, we've decided to move them into their directory main navs. As a result, each component navigation will now display 3 links for the logged in user :

• All Groups / My Groups / Create a Group
• All Sites / My Sites / Create a Site

We are aware, this might not be the best positioning for every users, so we are also introducing two new filters to help these users to neutralize it and choose their prefered positioning.

• 'bp_get_group_create_nav_item'
• 'bp_get_blog_create_nav_item'

Neutralizing the positioning can be achieve by simply returning false to these filters, here's an example for the Groups component create link.
add_filter( 'bp_get_group_create_nav_item', '__return_false' );

See #6008 / #4638

Props hnla, johnjamesjacoby, DJPaul, boonebgorges, modemlooper, imath