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

navigator.epubReadingSystem injected in EPUB content documents with delay #41

Closed
danielweck opened this issue Jul 10, 2014 · 12 comments
Closed
Assignees

Comments

@danielweck
Copy link
Member

Originally posted by @codingisacopingstrategy
at
readium/readium-js-viewer#13

@jccr
Copy link
Member

jccr commented Jul 14, 2014

I have pushed a branch with a solution to this:
feature/iframe_injection

@danielweck
Copy link
Member Author

Thanks!
This "solution" is elegant, but the navigator.epubReadingSystem object is
JSON-stringified (copied), therefore the actual instance does not
get shared amongst iframe-hosted EPUB HTML content documents. Am I right?
This is problematic, as any navigator.* object reference is meant to be
shared. We need navigator.epubReadingSystem to reflect a shared state (for
future use with widgets, etc.).
dan

On Monday, July 14, 2014, Juan Corona notifications@github.com wrote:

I have pushed a branch with a solution to this:
feature/iframe_injection


Reply to this email directly or view it on GitHub
#41 (comment)
.

@jccr
Copy link
Member

jccr commented Jul 14, 2014

@danielweck
Yes, the epubReadingSystem is serialized into "source" text. This is how it's made available from the very start. This is not necessarily limited to JSON, as you can see by my attempt to pass along simple functions.

I'll see if I can pass along an actual instance before the iframe document is written to. (And how something like this behaves across multiple browser engines.)

@jccr
Copy link
Member

jccr commented Jul 14, 2014

It looks like by just simply doing

        iframe.contentWindow.document.open();
        iframe.contentWindow.navigator.epubReadingSystem = navigator.epubReadingSystem;
        iframe.contentWindow.document.write(htmlText);

Achieves just that.

@danielweck
Copy link
Member Author

That would be great!
We now need to check all the features that depend on proper XHTML serialisation, like namespace-aware CSS selectors, annotations and Media Overlays -injected content, etc. We also need to thoroughly test on native apps, as the UIWebView tends to behave very differently in SDKLauncher-iOS/OSX/Android compared to pure browser implementations, especially when dealing with XHTML (e.g. self-closing tags).

@danielweck
Copy link
Member Author

@danielweck
Copy link
Member Author

I am wondering whether the document.open()+write() approach has an impact on "programmatic fetching" (document / file Blobs, etc.) See readium/readium-js#54

@jccr
Copy link
Member

jccr commented Jul 15, 2014

I believe it will.. It calls the basic iframe loader's loadIframe with an "empty" blob document as a source. This will throw everything off. I'm still unsure on how to test the use of the zipIframeLoader. Is it used in the chrome extension? Is it at all used from the cloud reader?

If the document.open()+write() approach is found to work on both loaders then we should refactor them to use the same approach.

@mickael-menu-mantano
Copy link

I tried a similar approach – albeit not using document.open() + write() – to inject JavaScript before the page's JavaScript is executed and faced a problem when changing the spine item in fixed layout books. The first time we set the href of the iframe it worked properly but the second time (next spine item), my JavaScript wasn't executed. I had to recreate a new iframe for each chapter for the solution to work, but it introduced other bugs in the layout when rotating the screen.

It seems very similar to the issue Juan talked about on the mailing-list, in the mail entitled "[AppJS] Readium + MathJax & Iframe injection":

Iframes need to be destroyed and recreated on every content document change. Iframe elements once rendered cannot be re-used when switching documents, or else MathJax will not reinitialize and render MathML after the switch. This still needs to be implemented for all other views, not just reflowable views.

Did you check if the epubReadingSystem was still available when changing the spine item in a fixed-layout?

@danielweck
Copy link
Member Author

I remember having conversations with Boris about the pitfalls of "iframe
recycling". I'll be following this discussion very closely. Dan

On Wednesday, July 16, 2014, Mickaël Menu notifications@github.com wrote:

I tried a similar approach – albeit not using document.open() + write() –
to inject JavaScript before the page's JavaScript is executed and faced a
problem when changing the spine item in fixed layout books. The first time
we set the href of the iframe it worked properly but the second time (next
spine item), my JavaScript wasn't executed. I had to recreate a new iframe
for each chapter for the solution to work, but it introduced other bugs in
the layout when rotating the screen.

It seems very similar to the issue Juan talked about on the mailing-list,
in the mail entitled "[AppJS] Readium + MathJax & Iframe injection":

Iframes need to be destroyed and recreated on every content document
change. Iframe elements once rendered cannot be re-used when switching
documents, or else MathJax will not reinitialize and render MathML after
the switch. This still needs to be implemented for all other views, not
just reflowable views.

Did you check if the epubReadingSystem was still available when changing
the spine item in a fixed-layout?


Reply to this email directly or view it on GitHub
#41 (comment)
.

@danielweck
Copy link
Member Author

As verified by @aadamowski, document.write() breaks XML namespaces, see:
readium/readium-js#54

@danielweck
Copy link
Member Author

Fixed in
#76

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

No branches or pull requests

3 participants