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

Chrome-specific iframe_loader strategy #142

Closed
danielweck opened this issue Aug 6, 2014 · 4 comments
Closed

Chrome-specific iframe_loader strategy #142

danielweck opened this issue Aug 6, 2014 · 4 comments
Assignees

Comments

@danielweck
Copy link
Member

Pre-process HTML (before storing into HTML filesystem) to inject the Javascript epubReadingSystem "bootstrapper" and MathJAX html>head>script

https://github.com/readium/readium-shared-js/blob/develop/js/views/iframe_loader.js#L179

This would negate the need for binary Blob URI iframe@src, thereby
allowing us to skip html>head>base@href and xml:base ...and as a byproduct: regain full SVG support, see:
readium/readium-shared-js#90

The "cloud reader" configuration would continue using the Blob URI mechanism introduced by:
readium/readium-shared-js#76

Email discussion:
https://groups.google.com/forum/#!msg/readium-dev/MtLL2xe4bcQ/ItUynlrqL-0J

@danielweck
Copy link
Member Author

Also see:
readium/SDKLauncher-OSX#10

@danielweck
Copy link
Member Author

@ryanackley Sorry, for the duplicated email message ( https://groups.google.com/forum/#!topic/readium-dev/nRLsBopCKnQ ), but this needs to be in the issue tracker too:

Before we merge "feature/chromeLoader" to "develop":

The refactoring of readium-shared-js "basic iframe loader" breaks the "zip loader" of readium-js, because the signature of the _loadIframeWithDocument() method does not have an actual document parameter anymore (and obviously, the creation of the binary Blob URL was removed, etc.):

readium/readium-shared-js@8ab08bc

https://github.com/readium/readium-js/blob/develop/epub-modules/epub-fetch/src/models/iframe_zip_loader.js#L37

However, I totally agree with your refactoring in principle: the shared-js "basic iframe loader" should really just be a basic one, i.e. (1) wire iframe@onload callback, (2) set iframe@src URI, (3) profit.

Note: thanks to the aforementioned refactoring, I will be able to remove the custom "iframe loader" which I implemented for ReadiumSDK launcher OSX, in the "htmlInject" feature branch:

https://github.com/readium/SDKLauncher-OSX/tree/feature/htmlInject
readium/SDKLauncher-OSX#10

So, I would suggest:

(1) [higher priority] that the original "iframe loader" from readium-shared-js be merged into the "zip iframe loader" code of readium-js. This way, all the cloud-reader -specific code goes where it belongs (i.e. readium-shared-js then effectively becomes the lowest common denominator for both ReadiumSDK native launcher apps, and ReadiumJS browser-based apps).

(2) [lower priority] that the "zip iframe loader" of readium-js not be used in the context of the Chrome extension, as EPUBs are always exploded into the HTML5 Filesystem prior to loading. It's a relatively trivial fix: just as we already do in readium-shared-js, use an option parameter at initialisation time that specifies a custom "iframe loader". The Chrome app/extension configuration would pass ReadiumSharedJS.BasicIFrameLoader, whereas the cloud reader would pass ReadiumJS.ZIPIFrameLoader. Note that as readium-shared-js does not use require.js, it's a basic Javascript "option" parameter:

https://github.com/readium/readium-shared-js/blob/develop/js/views/reader_view.js#L67

ryanackley added a commit that referenced this issue Aug 12, 2014
@ryanackley
Copy link
Contributor

Should be fixed

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

2 participants