WordPress.org

Make WordPress Core

Opened 11 years ago

Last modified 4 weeks ago

#13867 assigned enhancement

New filter for comment RSS feed's title

Reported by: shidouhikari Owned by: killua99
Milestone: 5.9 Priority: normal
Severity: normal Version: 3.0
Component: Feeds Keywords: has-patch has-unit-tests dev-feedback
Focuses: Cc:

Description

I'd like to be able to customize comments titles in RSS feed.

Currently it's hardcoded and has no way to be changed, so I added 2 new filters so that plugins can edit them.

I've tested and patch is working for me.

Attachments (8)

feed-rss2-comments.php.diff (1.9 KB) - added by shidouhikari 11 years ago.
Adds 2 filtters to allow plugins to edit comments titles in RSS feed
13867.diff (2.6 KB) - added by DrewAPicture 7 years ago.
+ hook docs
13867.2.diff (2.6 KB) - added by stevenkword 6 years ago.
updates hook doc
13867.3.patch (2.8 KB) - added by killua99 4 years ago.
13867.3.diff (2.9 KB) - added by telmoteixeira 3 years ago.
13867.4.diff (3.1 KB) - added by telmoteixeira 3 years ago.
13867.4.test.diff (2.6 KB) - added by telmoteixeira 3 years ago.
13867.5.diff (7.2 KB) - added by audrasjb 4 weeks ago.
Feeds: Add a hook to filter RSS2 and ATOM Comments feeds page title

Download all attachments as: .zip

Change History (30)

@shidouhikari
11 years ago

Adds 2 filtters to allow plugins to edit comments titles in RSS feed

#1 @nacin
11 years ago

  • Keywords early removed
  • Milestone changed from 3.1 to Future Release

These can already be filtered through gettext filters.

#2 @shidouhikari
11 years ago

  • Cc shidouhikari added

As we talked in IRC, using gettext one would need to test for 'Comment on %1$s by %2$s' and 'By: %s'...

'By: %s' isn't something very exact for title filtering. A specific filter is simpler to handle and has no risk of other 'By: %s' somewhere else being messed. Also, if in a future version one of those texts are changed, anything relying on them would get broken.

With a specific filter, just let core do its work and then filter the result.

And sorry about the early, I thought it means patches in an early new milestone :p

#3 @nacin
11 years ago

  • Milestone changed from Future Release to Awaiting Triage
  • Type changed from feature request to enhancement

Per IRC.

#4 @nacin
11 years ago

  • Milestone changed from Awaiting Triage to Future Release

#5 @chriscct7
7 years ago

Patch still looks good

#6 @DrewAPicture
7 years ago

  • Keywords needs-docs needs-patch added; has-patch removed

A few things here:

  • Both filters could probably use better names
  • Both would need hook documentation for consideration
  • As we need to make sure whatever output is passed through ent2ncr() it might be better to do it just-in-time when the filter value is output, and remove the others from the existing sprintf lines.

@DrewAPicture
7 years ago

+ hook docs

#7 follow-up: @DrewAPicture
7 years ago

  • Keywords has-patch added; needs-docs needs-patch removed
  • Milestone changed from Future Release to 4.2

Seems like with all of the RSS filters we already have, we'd have one or two in place we could reuse, but I'm not really seeing viable alternatives to the ones proposed in the patch.

13867.diff refreshes the 5-year-old patch and adds hook docs. Moving to 4.2 for consideration.

#8 in reply to: ↑ 7 ; follow-up: @stevenkword
7 years ago

Drew, I'm taking a serious look at the way we are applying filters to RSS. I'm finding that we are using too many catchall functions, and I think we need to re-examine the XML and RSS syntax rules. I'm currently putting together a proposal that relates to this ticket.

I'm currently thinking we need to have filters for individual encodings such as proposed in #19998, but I want to hold further comment until I have my head better wrapped around the big picture.

Last edited 7 years ago by SergeyBiryukov (previous) (diff)

#9 in reply to: ↑ 8 @mdgl
7 years ago

Replying to stevenkword:

Drew, I'm taking a serious look at the way we are applying filters to RSS. I'm finding that we are using too many catchall functions, and I think we need to re-examine the XML and RSS syntax rules. I'm currently putting together a proposal that relates to this ticket.

I agree our whole approach to encoding XML/RSS could do with a bit of a re-vamp, probably on a new ticket. It's actually quite a tricky problem and for a long time we have tended just to throw CDATA blocks around until things appear to work.

See #3670 for the beginnings of some ideas about a more general esc_xml() abstraction. Also related #28816.

This ticket was mentioned in Slack in #core by drew. View the logs.


7 years ago

#11 @ocean90
7 years ago

  • Milestone changed from 4.2 to Future Release

What would be an example use-case for this filter?

#12 @stevenkword
6 years ago

@ocean90 -- Changing the feed title to something that doesn't match the existing phrasing. e.g.) "Comments on: Hello World!" to something like "What People are Saying about Hello World!"

13867.diff still applies cleanly and 13867.2.diff updates the hook doc. I think this is ready to go.

@stevenkword
6 years ago

updates hook doc

#13 @stevenkword
5 years ago

  • Keywords needs-refresh added; has-patch removed

Patch no longer applies cleanly.

#14 @stevenkword
4 years ago

  • Keywords good-first-bug added

@killua99
4 years ago

#15 @killua99
4 years ago

Updating the code till the latest trunk

#16 @dingo_bastard
4 years ago

  • Keywords has-patch added

#17 @DrewAPicture
4 years ago

  • Owner set to killua99
  • Status changed from new to assigned

Looks like the attached patch here will need another refresh. Assigning this good-first-bug ticket to mark it as "claimed".

#18 @telmoteixeira
3 years ago

  • Keywords needs-unit-tests added; needs-refresh removed

Patch refreshed

#19 @telmoteixeira
3 years ago

  • Keywords has-unit-tests added; needs-unit-tests removed

After a closer look, created a new patch and added a unit test.

This ticket was mentioned in Slack in #core by sergey. View the logs.


3 months ago

#21 @SergeyBiryukov
3 months ago

  • Milestone changed from Future Release to 5.9

@audrasjb
4 weeks ago

Feeds: Add a hook to filter RSS2 and ATOM Comments feeds page title

#22 @audrasjb
4 weeks ago

  • Keywords dev-feedback added; good-first-bug removed

In 13867.5.diff, I partially rewrote the previous patch:

  • better function naming
  • only address main feed title, and not item titles to keep the ticket in a smaller scope
  • rewrote Unit tests
  • add support for ATOM feeds
  • add unit tests for ATOM feeds
Note: See TracTickets for help on using tickets.