johnfactotum / foliate-js

Render e-books in the browser
https://johnfactotum.github.io/foliate-js/reader.html
MIT License
418 stars 60 forks source link

Fixes corrupted/missing blobs aborting render #18

Open ojaskavathe opened 9 months ago

ojaskavathe commented 9 months ago

Fixes #14

Catch errors on the promise returned by entry.getData in loadBlob, and show a placeholder image in the event that the promise becomes rejected.

I made the placeholder so as to avoid any weird issues, let me know if it's too much and I'll edit the PR.

I also realized that if a chapter is mentioned in content.opf, but isn't actually provided, the whole render process is aborted when the user gets to that chapter and the app just freezes up. As such, I'm catching errors in loadText() and returning an empty blob if it's not present, which means at the very least rendering doesn't bork itself.

placeholder-screenshot-text

johnfactotum commented 9 months ago

Thanks. Though I'm still unsure whether this would be the best way to handle missing resources.

As for missing chapters, I would expect such errors to be caught when calling the load() method.

Another idea is that the book interface should have some way of allowing the renderer to transform the contents in some way while loading them. This would solve the more general problem, but also might give you a chance to handle the case of missing resources. Well, it's just an idea. Mostly I'd just like to move the paginator specific CSS replacements out of the epub module somehow.

ojaskavathe commented 9 months ago

Yeah it's probably better to handle missing resources in the books interface, rather than this band-aid fix.

Just so I'm understanding it correctly, you're proposing something like a transform.js that gets imported by the books interface(epub.js, fb2.js etc), and allows for them to modify the contents of books while loading resources, but before rendering them?

I could definitely work on something like that, although I'm a little lost about the paginator specific CSS replacements. I see that epub module replaces some CSS in loadItem, but I don't understand where the paginator fits into all of this.

I'm new to contributing in general, apologies if I sound all over the place.

johnfactotum commented 8 months ago

Yeah it's probably better to handle missing resources in the books interface, rather than this band-aid fix.

Well, there are two slightly different cases regarding "missing resources". The more general case is that whatever that is calling the load() method (either the Renderer or some higher level class) would need to handle the error.

The more specific case is that the EPUB class doesn't handle the actual resource loading itself, but rather takes a "Loader" object. So there's a question of whether to handle the error in the Loader or in the EPUB class.

Just so I'm understanding it correctly, you're proposing something like a transform.js that gets imported by the books interface(epub.js, fb2.js etc), and allows for them to modify the contents of books while loading resources, but before rendering them?

I'm thinking of a function, that can be passed to the Book interface. Either as an argument to the load() method, or to the constructor (alternatively, since there's currently no defined interface for the constructor, it would be up to each implementation whether or how to support such a feature).

I'm a little lost about the paginator specific CSS replacements. I see that epub module replaces some CSS in loadItem, but I don't understand where the paginator fits into all of this.

Some of the replacements are workarounds specific to the multicolumn pagination technique, which you might not want if you're not rendering that way. E.g. it replaces page breaks with column breaks. And such workarounds might be needed for non-EPUB formats as well. So ideally it should be handled by the Renderer.