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

Site /feed.xml is RSS 2.0, fix auto-discovery header #3176

Merged
merged 1 commit into from Dec 2, 2014

Conversation

Projects
None yet
4 participants
@rud
Contributor

rud commented Dec 1, 2014

Fixes a small issue reported on #2996, where the RSS auto-discovery header in a generated site is linked as type="application/atom+xml" where the /feed.xml file is in fact in RSS 2.0 format and should be type="application/rss+xml".

The page http://technotes.iangreenleaf.com/posts/2013-10-17-rss-the-least-wrong-way.html seems like a neat reference to double-check these values.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Dec 1, 2014

Member

The article seems to indicate this patch invalid:

Given what I just said about Atom feeds, it seems like application/rss+xml would be a better idea. However, this is not a registered MIME type, and while it will probably work, you still should not try it. RSS suffered greatly at the hands of those who loved it, and this content-type mess is one of the greatest legacies of that. Other content types used for RSS in the past have included application/xml, text/html, and text/rss+xml, which are all wrong and should be avoided.

What say you?

Member

parkr commented Dec 1, 2014

The article seems to indicate this patch invalid:

Given what I just said about Atom feeds, it seems like application/rss+xml would be a better idea. However, this is not a registered MIME type, and while it will probably work, you still should not try it. RSS suffered greatly at the hands of those who loved it, and this content-type mess is one of the greatest legacies of that. Other content types used for RSS in the past have included application/xml, text/html, and text/rss+xml, which are all wrong and should be avoided.

What say you?

@rud

This comment has been minimized.

Show comment
Hide comment
@rud

rud Dec 2, 2014

Contributor

Thank you for looking into this, much appreciated. You're absolutely right this is not a registered mime-type, however I based this change on the information a bit further down in the article under the "Auto-discovery" heading, where it says somewhat flippantly:

The type attribute is also important. It must be either application/atom+xml or application/rss+xml. “But wait,” you say, “why are we using application/rss+xml when you just told me that’s not a valid content type?” You’re so cute with your questions. But seriously, don’t use it in the content type header, do use it here, stop asking questions and no one will get hurt.

The the examples further down are also good. For instance, on wordpress.com they use this:
<link rel="alternate" type="application/rss+xml" title="WordPress.com Freshly Pressed" href="http://freshlypressed.wordpress.com/feed/">
for a RSS 2.0 feed.

In short, I think my PR is sound and correctly mimics the consensus on how RSS 2.0 auto-discovery links should be formatted. Feedback is still most welcome, though.

Contributor

rud commented Dec 2, 2014

Thank you for looking into this, much appreciated. You're absolutely right this is not a registered mime-type, however I based this change on the information a bit further down in the article under the "Auto-discovery" heading, where it says somewhat flippantly:

The type attribute is also important. It must be either application/atom+xml or application/rss+xml. “But wait,” you say, “why are we using application/rss+xml when you just told me that’s not a valid content type?” You’re so cute with your questions. But seriously, don’t use it in the content type header, do use it here, stop asking questions and no one will get hurt.

The the examples further down are also good. For instance, on wordpress.com they use this:
<link rel="alternate" type="application/rss+xml" title="WordPress.com Freshly Pressed" href="http://freshlypressed.wordpress.com/feed/">
for a RSS 2.0 feed.

In short, I think my PR is sound and correctly mimics the consensus on how RSS 2.0 auto-discovery links should be formatted. Feedback is still most welcome, though.

@mattr-

This comment has been minimized.

Show comment
Hide comment
@mattr-

mattr- Dec 2, 2014

Member

Please remove your change to History.markdown. Jekyll's team maintains that file so that you don't have to.

Once that's done we'll get this merged in.

Member

mattr- commented Dec 2, 2014

Please remove your change to History.markdown. Jekyll's team maintains that file so that you don't have to.

Once that's done we'll get this merged in.

@rud

This comment has been minimized.

Show comment
Hide comment
@rud

rud Dec 2, 2014

Contributor

Sounds good, was not aware of changelog discipline. Updated PR coming up in a few hours.

Contributor

rud commented Dec 2, 2014

Sounds good, was not aware of changelog discipline. Updated PR coming up in a few hours.

@rud

This comment has been minimized.

Show comment
Hide comment
@rud

rud Dec 2, 2014

Contributor

Patch amended, removed changes to History.markdown. Good to go with this?

Contributor

rud commented Dec 2, 2014

Patch amended, removed changes to History.markdown. Good to go with this?

@parkr parkr removed the Pending Comments label Dec 2, 2014

@parkr parkr merged commit 2c85f4e into jekyll:master Dec 2, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Dec 2, 2014

Member

Thanks!

Member

parkr commented Dec 2, 2014

Thanks!

parkr added a commit that referenced this pull request Dec 2, 2014

@jekyll jekyll locked and limited conversation to collaborators Feb 27, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.