search

lbryio / lbry-desktop

A browser and wallet for LBRY, the decentralized, user-controlled content marketplace.
https://lbry.tech
MIT License
3.56k stars 413 forks source link

PDF viewer in app (not external link) #2903

Closed tzarebczan closed 3 years ago

tzarebczan commented 5 years ago

We used to be able to open PDFs right inside the app and I believe it broke with one of the Chrome/Electron updates. We should investigate if this is possible when we upgrade Electron.

A user was recently not happy with this behavior and wrote up a script to support pdfs in .lbry files, but I don't think that's the right way to go - it should be possible to open them up right in the app. https://www.reddit.com/r/lbry/comments/d5uxnd/let_your_users_read_pdf_ebooks_within_lbry_app_no/

jeffslofish commented 4 years ago

Looking into this...

kauffj commented 4 years ago

Oops, I actually researched this when doing some weekend hacking and never updated the ticket.

I believe if you update the core Electron dependency that this will work again.

jeffslofish commented 4 years ago

The PDF issue in Electron was fixed in 9.0.0, which was recently released. There are some deprecations and other changes that require more changes in the code than just setting the Electron dependency to 9.0.0.

Here is what I did to get PDFs to work in the app:

  1. Update core Electron dependency to 9.0.0
  2. Set the nodeIntegration option to true (because it now defaults to false) under webPreferences here: https://github.com/lbryio/lbry-desktop/blob/master/electron/createWindow.js#L36
  3. Remove PDF from the const UNSUPPORTED_IN_THIS_APP here: https://github.com/lbryio/lbry-desktop/blob/master/ui/constants/file_render_modes.js#L30
  4. In lbry-redux, add mode: 'no-cors', to the apiCall options, here: https://github.com/lbryio/lbry-redux/blob/master/src/lbry.js#L186

Setting the mode to no-cors seems like a bad thing to do, but I couldn't find any way around it. Perhaps there needs to be a change to the lbry-sdk to make cors work with the latest electron?

This is what happens if the mode isn't set to 'no-cors':

405 method not allowed

I could submit a pull request with these changes (actually two pull request because some changes are in this repo and one is in lbry-redux) if you like, but I figured you might want to discuss this further before pulling in the latest version of Electron and making these changes.

neb-b commented 3 years ago

Electron has been updated to version 9 so we should be able to fix this now.

jeffslofish commented 3 years ago

Hi! It has been a while, but I am going to try to get setup again and work on this issue now.