Opened 14 years ago
Closed 14 years ago
#2598 closed defect (bug) (fixed)
[patch] Forum topic title should be included in bp_get_page_title()
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 1.5 | Priority: | normal |
Severity: | Version: | ||
Component: | Forums | Keywords: | has-patch, needs-testing |
Cc: |
Description
There's some code by Boone in an old forum topic that could easily be added to address this issue:
http://buddypress.org/community/groups/how-to-and-troubleshooting/forum/topic/editing-group-and-forum-page-title-tags/#post-36358
If there's no time for this, feel free to bump to 1.3.
Attachments (5)
Change History (37)
#2
@
14 years ago
- Component changed from Core to Forums
Boy, nothing comes easy, does it?
That code doesn't work anymore as written. Something changed around 1.2.3ish so that the $forum_template global is no longer populated around the time when BP is creating the page title. (I seem to remember some priorities changing around that time, anyway.) That means that bp_get_the_topic_title() was returning empty.
The attached patch takes a different swing at things, by putting some information about the current topic into the $bp global. It's not ideal, because it requires pulling up the topic an extra time, and because it relies on a bbPress function call. But it's more efficient (probably) then calling up the entire $forum_template twice, at least.
#3
@
14 years ago
- Keywords has-patch needs-testing added
- Summary changed from Forum topic title should be included in bp_get_page_title() to Forum topic title should be included in bp_get_page_title() (has patch)
#4
@
14 years ago
I don't know about you... but the old code is still working for me fine on BP 1.2.5.2!
http://pastebin.com/1zdN7C7K
Can someone confirm? Plop in functions.php.
#5
@
14 years ago
Ray,
I added your pastebin directly above and it works fine. The order was reversed in the topic title. I.E. after your patch it is now XXXX/Group/Site and was originally Site/Group/XXXX.
Using Single Install, 1.2.5.2 and WP 3.01
#6
@
14 years ago
Boone that is odd that bp_get_the_topic_title() returns empty I have tested it in a function running from functions.php on 1.2.5.2 and it works perfectly
#7
@
14 years ago
Ugh, I know what was happening. I took out the bp_has_forum_topic_posts() check, and bbpress_init therefore wasn't firing. That's why bp_get_the_topic_title() was returning false. I suck. Here's another patch based on what r-a-y wrote (based on what I wrote).
#8
@
14 years ago
I should probably go eat lunch before I attempt any more patching :-D Correct patch attached as -3.patch.
#10
@
14 years ago
Haha, it happens to the best of us, Boone!
Re: http://trac.buddypress.org/ticket/2598#comment:5
As an addendum, perhaps we should look at reversing the order for the entire page title in BP 1.3. Makes sense to me from an SEO standpoint.
#11
@
14 years ago
r-a-y - Agreed on rethinking the page title order for 1.3. For this patch I kept with the default BP component order.
#12
@
14 years ago
Good point re reversing the order it should be the opposite way round which I forgot and have just changed. topic title if considered important needs to be first.
#14
@
14 years ago
- Keywords changed from has-patch needs-testing to has-patch, needs-testing
- Summary changed from Forum topic title should be included in bp_get_page_title() (has patch) to [patch] Forum topic title should be included in bp_get_page_title()
#15
@
14 years ago
For reference:
Blog category tags displayed as slug? i.e hyphans not removed in words
If bp_page_title() is given a clean up in 1.3 then this thread perhaps ought to be checked:
http://buddypress.org/community/groups/requests-feedback/forum/topic/problem-with-title-tag-in-category-and-tag/#post-69554
#17
@
14 years ago
My Opinion? Patch to Boone's 2958.3 patch (no justification in this holding things up)
This addresses the main point of no topic title which is the real issue and gives us:
Site Name | Groups | Group Name | Forum | Topic title - for title tag string
For me I would say it's perhaps too verbose and prefer a variation on Boon's forum thread example and will possibly still extract the function and add to it:
$title = str_replace( ' | Groups', , $title );
$title = str_replace( ' | Forum', '| Topic', $title );
But this is I think a matter of choice? up to the individual to decide if they wish or prefer something a little different.
Hopefully we will start building a library of snippets functions people can use when the new codex is in place.
As for my previous comment not sure whether this bp_page_title() should be added to 1.3 as an enhancement ticket with a view to checking through thoroughly? The question of the hyphen in post category titles is one I think will have to be tackled with a user function for time being, shouldn't be difficult once the correct cat object 'name' or 'nice name' is found, did have a quick go but hadn't really time to hunt through WP codex.
#20
@
14 years ago
- Priority changed from normal to minor
- Resolution fixed deleted
- Status changed from closed to reopened
With a view to hyphenated (slug) category name a quick? fix is to add:
$title = str_replace('-', ' ', $title);
`} else if ( is_category() ) {
$title = ( 'Blog | Categories | ' . ucwords( $wp_query->query_varscategory_name? ), 'buddypress' );
$title = str_replace('-', ' ', $title);`
This, imho, is trivial issue that ought to be looked at for 1.3 but mentioned by a user, ticket re-opened but ought to be moved to 1.3? Please feel free to close at will :)
#21
@
14 years ago
- Milestone changed from 1.2.6 to 1.3
Bumping to 1.3. Don't think we want to modify user entered strings if we can help it.
#22
@
14 years ago
Agreed best bumped to 1.3
'Modifying user string' it's not a question of modifying user string, is it? We are taking the slug and spitting it out to browser where text should be; in human readable / seo / assistive devise terms we expect 'This is a title' rather than 'this-is-a-title'
#23
@
14 years ago
For localization purposes we can't rely 100% on the slug translating to the description.
I agree that this does need some tlc though.
#24
@
14 years ago
Take your point but had thought that simply preg_replace /str_replace / ereg_ or whatever on the hyphens to white space wouldn't have caused a localization issue.
But regardless yes a bit of tlc when appropriate in 1.3. This isn't a huge big issue but one that could be irritating to users and not great from SE description perspective.
#25
@
14 years ago
2598-4.patch is a less hackish solution for category archive pages. The only downside is that it means an additional query. FWIW WP itself does something similar in wp_title().
Thoughts?
#26
@
14 years ago
2598-5.patch allows for localisation. The entire function needs to be re-written in the same way.
#27
@
14 years ago
Have a read of this -- http://daringfireball.net/2010/12/title_junk
I agree with Mr Gruber's "Source: Headline" preference.
#28
@
14 years ago
Kind of off-topic, but I totally agree with that article.
It's a throwback to using the <title> tag for what it was originally intended; though I do prefer "Headline - Source" myself.
I say this because if all listings on Google were like "Source: Headline" and you're making a search on one domain only (eg. "site:domain.com" on Google), it would make it particularly hard to find the page you want to view.
#29
@
14 years ago
- Owner set to DJPaul
- Priority changed from minor to normal
- Status changed from reopened to assigned
FWIW if this doesn't get added I will be having to preempt what I know will be a query raised by the other developers on a BP implementation I'm working on; their SEO*sigh* chap will leap on this as a major issue and be asking me to correct it, I would rather not let the apparent flaw be commented on so will have to implement Boone's solution in functions.php