New filter for comment RSS feed's title

[shidouhikari]

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.

[shidouhikari]

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

[nacin]

These can already be filtered through gettext filters.

[shidouhikari]

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

[nacin]

Per IRC.

[chriscct7]

Patch still looks good

[DrewAPicture]

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]

+ hook docs

[DrewAPicture]

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.

[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'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.

[mdgl]

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.

[ocean90]

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

[stevenkword]

@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]

updates hook doc

[stevenkword]

Patch no longer applies cleanly.

[killua99]

Updating the code till the latest trunk

[DrewAPicture]

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

[telmoteixeira]

Patch refreshed

[telmoteixeira]

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

[audrasjb]

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

[audrasjb]

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

[hellofromTonya]

Thank you @audrasjb for updating and improving the patch.

I'm wondering: should the filtered titles, i.e. $feed_title, be escaped before echoing?

[audrasjb]

Given <title> doesn't allow any HTML tag, I think you're right @hellofromTonya. Patch incoming in two mins :)

[audrasjb]

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

[audrasjb]

I think we're good to go now.

[hellofromTonya]

The tests are failing missing test method:

1) Tests_Feed_Atom::test_feed_title
Error: Call to undefined method Tests_Feed_Atom::do_rss2_comments()

/var/www/tests/phpunit/tests/feed/atom.php:151

2) Tests_Feed_RSS2::test_feed_title
Error: Call to undefined method Tests_Feed_RSS2::do_rss2_comments()

/var/www/tests/phpunit/tests/feed/rss2.php:143

Removing commit and adding needs-refresh.

Trac ticket: https://core.trac.wordpress.org/ticket/13867

[audrasjb]

Wrong ticket, removing my comment

[audrasjb]

I addressed the missing tests issue in the above PR.
Marking this for commit.

[peterwilsoncc]

I'm going to bump this to a future release given the feature freeze today.

As noted on the PR:

• there's an unrelated change to this ticket's purpose
• I have some questions about the order of operations re: escaping.

There's no 6.0 milestone as yet but I think this is pretty close so can probably be added once the milestone is set up on trac.

---

Additional props due @costdev for feedback provided via slack.