Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reader Integration: Add ids to RSS feed #996

Closed
kraftbj opened this issue Aug 12, 2014 · 9 comments
Closed

Reader Integration: Add ids to RSS feed #996

kraftbj opened this issue Aug 12, 2014 · 9 comments
Assignees
Labels
General [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Milestone

Comments

@kraftbj
Copy link
Contributor

kraftbj commented Aug 12, 2014

Per @blowery, it would be helpful for the WordPress.com Reader if feeds from Jetpack sites included the blog ID as metadata in the feed to confirm that site is a Jetpack site.

This would aid the Reader to understand if any of the WP.com data can be trusted or used for hints or if a site was previously a JP site, now no longer.

@blowery
Copy link
Contributor

blowery commented Aug 12, 2014

Yup. Was thinking something like <site xmlns="com-wordpress:feed-additions:1">1234</site> inside the rss/channel element for an RSS feed and the feed element of an Atom feed. The namespace here is something I just made up, so feel free to use something more sane.

I'd rather have the data inside the actual feed rather than as a X-Jetpack-SiteId HTTP header because the feed contents should survive things like Feedburner and the like.

@blowery
Copy link
Contributor

blowery commented Aug 12, 2014

Additionally, having the post id in each item would be super handy. Something like <post-id xmlns="com-wordpress:feed-additions:1">1234</post-id>. Right now we're parsing the guid, which works most of the time, but something more explicit would be great.

@blowery blowery changed the title Reader Integration: Add id to RSS feed Reader Integration: Add ids to RSS feed Mar 10, 2015
@samhotchkiss samhotchkiss added this to the 3.6 milestone Jun 9, 2015
@dereksmart dereksmart modified the milestones: 3.6, 3.7 Jun 25, 2015
@samhotchkiss samhotchkiss modified the milestones: 3.7, Needs Triage Aug 28, 2015
@zinigor zinigor modified the milestones: 3.9, Needs Triage Dec 1, 2015
@jeherve jeherve modified the milestones: 4.0, 3.9 Jan 15, 2016
@kraftbj kraftbj self-assigned this Jun 9, 2016
@kraftbj kraftbj modified the milestones: 4.2, 4.1 Jun 9, 2016
@kraftbj
Copy link
Contributor Author

kraftbj commented Jun 9, 2016

I'm going to take this on, aiming for 4.2.

@jeherve jeherve modified the milestones: 4.3, 4.2 Jul 6, 2016
@richardmuscat richardmuscat modified the milestones: 4.3, 4.4 Jul 7, 2016
@sdellis
Copy link

sdellis commented Sep 28, 2016

@blowery wrote this, but it doesn't seem like anyone took him up on the advice:

The namespace here is something I just made up, so feel free to use something more sane.

The way the namespace is defined makes it the default namespace and can break parsers (as was my case with a Drupal module recently). Can this namespace be defined with a proper prefix? Something like:
xmlns:automattic="com-wordpress:feed-additions:1"

@kraftbj
Copy link
Contributor Author

kraftbj commented Sep 28, 2016

@sdellis I admit I know not nearly enough about XML Namespacing. Opened a new issue at #5219 for this.

@blowery
Copy link
Contributor

blowery commented Sep 29, 2016

The way the namespace is defined makes it the default namespace and can break parsers (as was my case with a Drupal module recently).

@sdellis An xmlns declaration only applies to the declaring element and that element's descendants. In our case, there are no descendants, so the namespace only applies to the site and post-id elements, not any other elements in the feed. If an xml parser is applying it as the default namespace up the element tree, that parser is in violation of the xml + namespaces spec...

Unless I'm missing something? Do you have an example showing how it broke something?

@sdellis
Copy link

sdellis commented Sep 29, 2016

My apologies. You are correct, @blowery, and this seems like a bug in the way the Drupal Feeds XPath Parser module determines default namespaces. Here's an example of where it's falling down in that module. Feel free to close #5219 @kraftbj. I will report the bug over there. Sorry for the false alarm, and thanks for your feedback!

@blowery
Copy link
Contributor

blowery commented Sep 29, 2016

@sdellis no worries! For a bit there I was really quite worried. It'd been a while since I read the XML 1.1 spec.

:D

@phubner
Copy link

phubner commented Jun 25, 2024

I know that this was has been rolled into the release already, but this change appears to make the mailchimp rss feed parser unhappy. Is there a way to disable this addition to the RSS feed without having to roll my own feed? The error MC support has pointed me to is the one reported in #35589 . While Google News parser seems better at handling slight errors, the MC one seems a bit less resilient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
General [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Development

No branches or pull requests

9 participants