readium / readium-shared-js

Repository for the shared JavaScript libraries that are used in the SDK-Launchers and other applications developed on top of the SDK
BSD 3-Clause "New" or "Revised" License
78 stars 102 forks source link

Using the id "cover" in the package file conflict with the generation of a default cover... #204

Open iherman opened 9 years ago

iherman commented 9 years ago

I have an EPUB file that does not have an explicit cover image. I expect Readium to come up with some sort of a default background possibly with the title overlaid on it. However, I found a problem with this.

Here is the package file content I had (omitting the uninteresting parts):

 <dc:title id="title">Generating JSON from Tabular Data on the Web</dc:title>
 …
</metadata>
<manifest>
 <item href="cover.xhtml" id="cover" media-type="application/xhtml+xml" />
 <item href="Overview.xhtml" id="main" media-type="application/xhtml+xml" />
 …
</manifest>
<spine toc="ncx">
 <itemref idref="cover" />
 <itemref idref="main" />
</spine>

if I use this, and I upload the book into Readium, there is no generated cover page at all; it is blank. However, if I use the following package file content:

<metadata>
 <dc:title id="title">Generating JSON from Tabular Data on the Web</dc:title>
 …
</metadata>
<manifest>
 <item href="cover.xhtml" id="start" media-type="application/xhtml+xml" />
 <item href="Overview.xhtml" id="main" media-type="application/xhtml+xml" />
 …
</manifest>
<spine toc="ncx">
 <itemref idref="start" />
 <itemref idref="main" />
</spine>

Then it works. The difference: using start as an id value in the <item> element as well as an idref in the spite, instead of cover. I think this is a bug, there should be no special value on an id value...

(I think this bug is actually similar to #203 insofar as using a non-prefixed term for internal purposes.)

danielweck commented 9 years ago

Thank you for reporting this @iherman Looking at the code, id="cover" is a "dumb" fallback for EPUB2 ebooks (but unfortunately, this is probably a necessary hack for a lot of older e-books). See: https://github.com/readium/readium-js-viewer/blob/develop/src/js/PackageParser.js#L55

getCoverHref : function(dom) {
            var manifest; var $imageNode;
            manifest = dom.getElementsByTagName('manifest')[0];

            // epub3 spec for a cover image is like this:
            /*<item properties="cover-image" id="ci" href="cover.svg" media-type="image/svg+xml" />*/
            $imageNode = $('item[properties~="cover-image"]', manifest);
            if($imageNode.length === 1 && $imageNode.attr("href") ) {
                return $imageNode.attr("href");
            }

            // some epub2's cover image is like this:
            /*<meta name="cover" content="cover-image-item-id" />*/
            // PragProg ebooks have two cover entries in meta, both
            // referencing the same cover id from items; metaNode.length
            // does not have to be just 1
            var metaNode = $('meta[name="cover"]', dom);
            var contentAttr = metaNode.attr("content");
            if(metaNode.length >= 1 && contentAttr) {
                $imageNode = $('item[id="'+contentAttr+'"]', manifest);
                if($imageNode.length === 1 && $imageNode.attr("href")) {
                    return $imageNode.attr("href");
                }
            }

            // that didn't seem to work so, it think epub2 just uses item with id=cover
            $imageNode = $('#cover', manifest);
            if($imageNode.length === 1 && $imageNode.attr("href")) {
                return $imageNode.attr("href");
            }

            // seems like there isn't one, thats ok...
            return null;
        },
    }

One way to fix this would be to add a check that id="cover" is indeed an image (check for the media-type attribute value).