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 cover_image #8

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

Conversation

rhaseven7h
Copy link

Fixed cover_image to retrieve cover_image information correctly.

…information,

     not backwards compatible, now returns a Hash in the likes of:
     { :file_name=>"cover.jpg", :media_type=>"image/jpeg", :file_contents=> "\xFF\xD8\xFF\xE0\x00\x10JFIF\x00\x01\..." }

FOX: Metadata.Metas now correctly returns content for <Meta> tags.
@KitaitiMakoto
Copy link
Owner

Thank you for the pull request.

But I have some things what I should say:

The first thing is about specification version. This library is EPUB 3 parser, but your implementation is the one for EPUB 2. I know there are so many EPUB 2 books, so it might be a good idea to fall back to EPUB 2 cover image when no EPUB 3 cover item is found. But I don't have any idea about how to implement it. Welcome if you will do it.

The second is about method you should patch. EPUB::Book::Features#cover_image is, as comment says, a syntactic sugar or shortcut method. You should patch EPUB::Publication::Package::Manifest#cover_image rather than Features.

Finally, please send a pull request after making your patch pass all the test. And, if you can, add tests to prevent development in the future from breaking your code. You can run tests by

$ bundle exec rake test

Are you interested in implementing to satisfy both EPUB 3 and 2 specificadtions? If so, continue in this pull request. If not, please close this issue.

Thanks!

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