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

fix #584 #630

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

fix #584 #630

wants to merge 7 commits into from

Conversation

antoniosanct
Copy link
Contributor

Hi, everyone:

I sent a new patch according to #584 because new 2.x master branch has come! Maybe it wouldn't be an issue. Please review and make your comments.

Regards,
Antonio.

antoniosanct and others added 3 commits February 23, 2023 23:18
Signed-off-by: Antonio Santos Izaguirre <antoniosanct@gmail.com>
@PatrickGotthard
Copy link
Member

PatrickGotthard commented Mar 11, 2023

Hi @antoniosanct,

I think the comment element was intentionally not set on SyndFeed. When converting

<?xml version="1.0" encoding="UTF-8"?>
<rss version="2.0">
    <channel>
        <title>Channel Title</title>
        <link>https://example.com</link>
        <description>Channel Description</description>
        <item>
            <title>Item Title</title>
            <link>https://example.com/item</link>
            <description>Item Description</description>
            <comments>https://example.com/comments</comments>
        </item>
    </channel>
</rss>

to Atom 1.0, the following XML will be produced:

<?xml version="1.0" encoding="UTF-8"?>
<feed xmlns="http://www.w3.org/2005/Atom">
  <title>Channel Title</title>
  <link rel="alternate" href="https://example.com" />
  <subtitle>Channel Description</subtitle>
  <entry>
    <title>Item Title</title>
    <link rel="alternate" href="https://example.com/item" />
    <link rel="comments" type="text/html" href="https://example.com/comments" />
    <author>
      <name />
    </author>
    <id>https://example.com/item</id>
    <summary type="html">Item Description</summary>
  </entry>
</feed>

According to RFC 4287 (The Atom Syndication Format) only IANA-registered link relations are allowed and there's simply no matching type for "comments".

Regards,
Patrick

@PatrickGotthard
Copy link
Member

Hi @antoniosanct,

any update about this? Seems that we produce invalid Atom feeds when merging this...

Regards,
Patrick

@antoniosanct
Copy link
Contributor Author

antoniosanct commented Mar 16, 2023

Hi, @PatrickGotthard

Hhmmm that's interesting... Then the test was bad implemented by this reason? If you think that the test is correct in his actual form, then close PR without merging it.

However, I only modified RSS 0.94 to pass the test. Does this behaviour affect the rest of converters? For example, TestSyndFeedAtom10.java @ rome use illegal rel values in atom_1.0.xml. If Rome controls which rel you can render, these tests (testEntry0, testEntry1) would be invalid by definition.

Regards,
Antonio.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants