Opened 12 years ago
Closed 12 years ago
#21233 closed defect (bug) (fixed)
Twenty Twelve: Make title truly pluggable
Reported by: | obenland | Owned by: | lancewillett |
---|---|---|---|
Milestone: | 3.5 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Bundled Theme | Keywords: | has-patch |
Focuses: | Cc: |
Description
I had a discussion with @mamaduka over at the Github repo about this topic (which is gone :( ) and it seems to be a controversial one.
WPTRT guidelines state:
wp_title()
: Themes are required to modify output via filter ('wp_title'
)
I'd like to see Twenty Twelve set a good example for Theme developers here. I have to flag (almost) every new Theme I review for that.
Attachments (4)
Change History (17)
#3
@
12 years ago
Let's not.
While #12370 is somewhat related, IMO #18548 is not really. In the iterative spirit from last night's dev chat, this ticket here would be more a "lets make the best from what we have" approach.
If any outcome from #18548 has repercussions on Twenty Twelve, let's deal with it at that time.
#5
follow-up:
↓ 6
@
12 years ago
Agree 100% with the filter, though I'd like to add more to it along the lines of what Twenty Eleven does right now (which should also be moved to a filter now).
http://core.trac.wordpress.org/browser/trunk/wp-content/themes/twentyeleven/header.php#L27
@obenland Want to update your patch with that extra output?
#6
in reply to:
↑ 5
;
follow-up:
↓ 7
@
12 years ago
Replying to lancewillett:
Agree 100% with the filter, though I'd like to add more to it along the lines of what Twenty Eleven does right now (which should also be moved to a filter now).
Should I create a new ticket for that? Or add a patch in #12370?
@obenland Want to update your patch with that extra output?
Sure thing, I hope you like it!
#7
in reply to:
↑ 6
@
12 years ago
- Keywords dev-feedback removed
Replying to obenland:
@obenland Want to update your patch with that extra output?
Sure thing, I hope you like it!
Patch is 99% there, thanks for that. I'm going to update the comments a tiny bit and remove an extra trailing whitespace near line 153.
#8
@
12 years ago
- Owner set to lancewillett
- Resolution set to fixed
- Status changed from new to closed
In [21276]:
#9
@
12 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
Since [21276], site title is displayed twice in RSS feed:
<title>WordPress TrunkWordPress Trunk</title>
In the feed files, wp_title_rss()
is already prepended with site title:
http://core.trac.wordpress.org/browser/tags/3.4.1/wp-includes/feed-rss2.php#L24
21233.3.diff fixes the duplication.
#10
follow-up:
↓ 11
@
12 years ago
Great catch, Sergey! The patch would be a quick fix, but wouldn't it make more sense to remove bloginfo_rss('name');
from the feed title?
We could add a filter to 'wp_title_rss'
or 'get_wp_title_rss'
instead and check if the title is empty, before adding the blog name.
Clearly not part of this ticket, but way more sustainable than coding around it, IMO.
What do you think?
#11
in reply to:
↑ 10
;
follow-up:
↓ 12
@
12 years ago
Replying to obenland:
Great catch, Sergey! The patch would be a quick fix, but wouldn't it make more sense to remove
bloginfo_rss('name');
from the feed title?
Sounds good, but I'm not sure about possible back compat repercussions for themes and plugins which already take into account that site title is hardcoded.
We could add a filter to
'wp_title_rss'
or'get_wp_title_rss'
instead and check if the title is empty, before adding the blog name.
Adding the blog name to empty titles only would mean that it will no longer be displayed for category feeds. I guess we should add it if it's not already present (while keeping the whole title filterable).
BTW, category and tag feed titles are also broken in current trunk:
<title>WordPress Trunk » helloWordPress Trunk</title>
Removing the prepended site title wouldn't fix the order, so 21233.3.diff might still be needed, unless we want to reimplement some of wp_title()
or try to filter out the separator.
Clearly not part of this ticket, but way more sustainable than coding around it, IMO.
I've created #21366 for removing the hardcoded site title.
#12
in reply to:
↑ 11
@
12 years ago
remove
bloginfo_rss('name');
from the feed title
Sounds good, but I'm not sure about possible back compat repercussions for themes and plugins which already take into account that site title is hardcoded.
Yes, but this is a catch 22: It's either back-compat problems or prepending the site title with a really late filter, which makes removing it pretty much obsolete.
Adding the blog name to empty titles only would mean that it will no longer be displayed for category feeds. I guess we should add it if it's not already present (while keeping the whole title filterable).
I agree. I like your patch in #21366.
Removing the prepended site title wouldn't fix the order, so 21233.3.diff might still be needed, unless we want to reimplement some of
wp_title()
or try to filter out the separator.
Right. I'd be fine with the patch, leaving the order as is.
Consider resolving this as part of these related tickets: #12370, #18548.