Skip to content

Conversation

x0000ff
Copy link
Contributor

@x0000ff x0000ff commented Oct 14, 2025

The OPDS 1.x is based on Atom, and Atom’s Feed may contain an icon tag. I found it useful to parse the <icon> tag.

1.1. Introduction
The Open Publication Distribution System (OPDS) Catalog format is a syndication format for electronic publications based on Atom [RFC4287] and HTTP [RFC2616].

atom:feed elements MUST NOT contain more than one atom:icon element.

  • ✅ Tests are updated

@x0000ff x0000ff changed the title Implement parsing of the <id> element in OPDS 1.x feeds Implement parsing of the <icon> element in OPDS 1.x feeds Oct 14, 2025
@x0000ff x0000ff marked this pull request as draft October 14, 2025 20:28
@mickael-menu
Copy link
Member

Thank you, this is a useful parsing addition.

Instead of adding a new property in the OpdsMetadata model, could you add the icon as a Link object in the Feed.links collection? This is how we handle external resources in our models.

You can add an icon LinkRelation in the Link object to mark it as the feed icon.

@x0000ff x0000ff force-pushed the feature/parse-feed-icon-for-opds-1.x branch from bac0256 to c8e72ec Compare October 15, 2025 17:12
@x0000ff
Copy link
Contributor Author

x0000ff commented Oct 15, 2025

Thank you, this is a useful parsing addition.

Instead of adding a new property in the OpdsMetadata model, could you add the icon as a Link object in the Feed.links collection? This is how we handle external resources in our models.

You can add an icon LinkRelation in the Link object to mark it as the feed icon.

Thank you, @mickael-menu!

I've update the PR. Could you check?

@x0000ff x0000ff marked this pull request as ready for review October 15, 2025 17:13
Copy link
Member

@mickael-menu mickael-menu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for making the changes @x0000ff! However, I can't merge it because it is not up to date with develop. Could you upgrade the branch or better yet, enable "edit from maintainers"? (see https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork#enabling-repository-maintainer-permissions-on-existing-pull-requests).

In your other PR as well #655

@x0000ff x0000ff force-pushed the feature/parse-feed-icon-for-opds-1.x branch from c8e72ec to e05b353 Compare October 21, 2025 13:36
@x0000ff
Copy link
Contributor Author

x0000ff commented Oct 21, 2025

Thanks a lot, @mickael-menu! I did both ✅

@mickael-menu mickael-menu merged commit 1aa5fac into readium:develop Oct 21, 2025
5 checks passed
@x0000ff x0000ff deleted the feature/parse-feed-icon-for-opds-1.x branch October 21, 2025 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants