search

NYPL-Simplified / webpub-viewer

8 stars 4 forks source link

Fixes internal links #173

Closed hokei closed 3 years ago

hokei commented 3 years ago

Internal links used to throw a 404 error.

This was because the webpub viewer assumed that the same server would serve both the parent and child of the iframe.
Because this is no longer the case:

1) Iframe srcdoc was modified to make sure that base referred to the site inside the iframe 2) Same origin checking was changed to ensure referrer and link href were the same, rather than window.location.

hokei commented 3 years ago

To Test: npm link library-simplified-webpub-viewer in Circulation-Patron-Web to test links inside epubs. Just a note: If you update webpub-viewer and want to see the changes, you have to run npm run build inside webpub-viewer to get it to show up.
I need to find a webpub that has links inside it, so will test that on my end -- I want to make sure this hasn't broken that. Ideally unit tests would catch that, but I'm not sure I fully trust the test coverage here.

kristojorg commented 3 years ago

Okay I'll give this a test asap then

EdwinGuzman commented 3 years ago

Should this work with the open-ebooks url, https://qa-circulation.openebooks.us/USOEI/groups/? I've having a hard time testing because the "read online" link opens a blank page rather than the reader, and get this message:

There was a problem fetching this book. Please refresh the page or return home.

Error Code: unknown
Error Message: Failed to parse OPDS data
hokei commented 3 years ago

How are you trying to test it? The OPDS feed urls only work if you're running it inside circulation-patron-web. on npm run streamed should still only work with webpub manifest urls (maybe direct links to epubs? I havne't tried that in a while ...), and npm run static should only work with local books (epubs or webpubs)

EdwinGuzman commented 3 years ago

Ah, do I need to start the server for the web reader? I was just running cpw but did npm link the package locally.

hokei commented 3 years ago

You shouldn't need to - If you npm link the webreader you should be able to start cpw with the open ebooks params and see the local webreader. If you're having issues msg me on slack and we can walk through it!

hokei commented 3 years ago

oh no! Let me take a look again, this is a pretty common issue that happens a lot when the reader state is weird.

hokei commented 3 years ago

OK I neatened up the link event handler, I think this should work.

kristojorg commented 3 years ago

I get this error when following an internal link on the book "Why Soccer Matters":

image

This is using the config branch in the PR I just opened. I will check if it also happens on beta... It looks like it is because there is a fetch request being sent to: https://localhost:3000/openqa/read/9780698150058_EPUB-5.xhtml... not sure if that's the problem or not.