johnfactotum / foliate-js

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

Add MutationObserver alongside ResizeObserver #11

Closed aehlke closed 11 months ago

aehlke commented 11 months ago

I'm using userscripts (foliate-js inside a webview in a swift app...) to manipulate the content of ebooks for educational purposes. This PR adds a mutation observer to trigger rendering when content is changed.

aehlke commented 11 months ago

sorry, this isn't ready - it's observing the wrong document. I'll update here when it's tested and working.

johnfactotum commented 11 months ago

Couldn't you listen to the custom event load on the renderer element (which has event.detail.doc) and observe it from there?

aehlke commented 11 months ago

I got it working by listening from the paginator class, but if I have trouble I'll look into that.

The current trouble is correcting the scroll position when the content shifts on mutation, will look into that next.

I must say after coming from epub.js this library is extremely nice to work with, good work.

johnfactotum commented 11 months ago

Should be fixed by ad25f6b5c3d9d044941d8ea858324b20a5352512. It shouldn't be necessary to observe for mutation if you're only looking for content resize, which should already be handled by the resize observer.

aehlke commented 11 months ago

Thanks very much, testing that change now. I'm not sure this will fix it because it seems the issue is that when I scroll quickly via arrow keys (on keyboard), I'm able to see the 2nd page of a section, but then the content mutates before the scroll event fires (I guess the scroll animation hasn't finished sufficiently, even though as a user it feels like I've already proceeded) and so the anchor is still set to the first page of the section causing it to jump back. I'm looking into how to get it to save the new anchor position before the animation fully completes.

edit: Indeed the patch doesn't resolve my above issue, and I should have clarified I'm talking about the paginated/column view. I'll update when I find a fix.

I'll look into eliminating the MutationObserver as well but that part is working now so I will revisit another time, appreciate the tip. It wasn't working for me on trunk, images reloaded with the wrong size and content needed re-columnization, but maybe there's still a way to avoid the mutation observer. Also, I did indeed put the observation inside the load function.

aehlke commented 11 months ago

I'm unable to resolve the scrolling jumping issue, so I'll abandon this approach for now and attempt to inject content modifications via loaders instead of after being loaded into the DOM.

aehlke commented 11 months ago

Are the changes in epub.js acceptable to you? I use it to send the changes to the host app for manipulation before rendering. I can clean up the PR if you'd accept this and look into adding more than just epub support. Please ignore the changes in the other files. Thanks.

johnfactotum commented 11 months ago

Well, for EPUB, you can just replace text in your loadText function.

aehlke commented 11 months ago

I do it this way to have the processing done in loadReplaced before I start transforming it further but maybe it's unnecessary for epub. I intend to add support for the other formats which IIRC have more necessary loadText post-processing

johnfactotum commented 11 months ago

It's not clear to me what your use case is exactly. I think transforming the content before replacing the resources is slightly more powerful in that it allows referencing resources by their original paths.

Another option, if you want to process it with the resources already replaced, would be to wrap epub.js. That is, you can create your own class that implements the book interface, and use epub.js internally, fetching the URL returned by load().

Another way is to transform the content from a load event handler, which would be the preferred method if you're working with the DOM.

aehlke commented 11 months ago

Use case: in this image you can see a result of my post-processing, where I segment Japanese text into individual words to allow clicking them to look up dictionary entries without requiring selecting the word boundaries manually; adding ruby text; annotating words with learner difficulty levels and highlighting words that are not yet known; and adding paragraph-ish segmenting for marking incremental reading progress and marking those individual words as read. These features require manipulating the DOM rather than using just an overlay without changing the design.

image

I think transforming the content before replacing the resources is slightly more powerful in that it allows referencing resources by their original paths.

^ I don't think I have a use case for this, and was worried about possibly needing something from the end result for the various formats, or transforming the doc in a way that breaks this resource replacement step for questionable value

Wrap epub.js: would need to investigate this more to think through

load event: I tried this method first, where the document is loaded into the iframe first and then manipulated, but it resulted in janky rendering that I failed to resolve and it was slower than this approach. so I'm now doing the DOM manipulation of the document inside my Swift layer before it's loaded into the iframe. it's working well for me now this way so I was hoping to avoid needing to keep a fork / thought this could be useful to others. thanks for your time.

johnfactotum commented 11 months ago

So, IIUC, when you transform it after it's loaded, the transformation isn't fully synchronous and/or you're transforming it after it's rendered, and consequently the result is janky? If so, perhaps you could try hiding it (e.g. with an opaque overlaid div) until the transformation is complete, at which point you could make it scroll to the desired location before showing it.

As to transforming the content before loading, I feel that wrapping would be a much neater solution as opposed to having to add transformation APIs to every format, but it would be less convenient and probably less performant, and maybe that's acceptable given that books are more usually rendered as is.

Another idea is that perhaps instead of focusing solely on transforming the content, the loaders could take a more general interface that takes care of the whole blob and blob URL creation process. Something along this line could e.g. give one more control over memory usage.

aehlke commented 11 months ago

Thanks for the feedback. I was unable to get the result to look correct with transforming after loading, notably the scroll behavior working reliably, so I abandoned that approach. Wrapping if less performant and more work is also not good for this as the processing already adds loading time, tho I appreciate it avoids complicating your API. Having the loaders use a more general API as you describe sounds good but not sure I'm ready to contribute that, so for now I will close this and maintain a fork.

BTW if you are available for contract work, in the future I would be interested in paying to extend this library with some functionality such as with continuous scrolling.