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

Fixes corrupted/missing blobs aborting render #18

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ojaskavathe
Copy link

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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.

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.

Chapter with broken images can't be rendered
2 participants