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

PDFs are downloaded (creating duplicates) rather than opening in the browser inline #763

Closed
cjohnsto-nz opened this issue Sep 16, 2021 · 9 comments

Comments

@cjohnsto-nz
Copy link

cjohnsto-nz commented Sep 16, 2021

Tested on Edge. When opening an attached PDF, it downloads a copy (1), (2), etc. The wiki I am accessing contains large technical documentation that users will be opening many times a day. These duplicates will quickly fill the entire computer.

Would it be possible to have the browser extensions attempt to open PDF files inline.
https://stackoverflow.com/questions/6293893/how-do-i-force-files-to-open-in-the-browser-instead-of-downloading-pdf

@cjohnsto-nz cjohnsto-nz changed the title Downloaded files, such as attached PDFs, duplicated each time they are accessed. PDFs are downloaded (creating duplicates) rather than opening in the browser inline Sep 17, 2021
@Jaifroid
Copy link
Member

Jaifroid commented Sep 17, 2021

@cjohnsto-nz In Kiwix JS we simply use the standard file download API, or else use the code for downloading files that is defined in the ZIM. As you know from kiwix/kiwix-js-pwa#194, it's a bit more complicated there because we have to use UWP APIs and we could (potentially) use the File System Access API in the PWA version (currently that is no different than it is in the Extension). I think that without using the File System Access API, we can't change the filename here in Kiwix JS. The API is unfortunately not supported by Firefox (it does work in Chrome and Edge). Supporting it is something we're considering -- see #656.

@cjohnsto-nz
Copy link
Author

I think I understand, but I am still a bit confused.

If the mime type gets picked up correctly as a PDF (or any other file type the browser can open inline) for displayFileDownloadAlert(), could you handle the file differently. I'm trying to piece together where you would add the Content-Disposition: inline; header, in order to open PDFs natively in the browser. This would avoid "downloading" them altogether, and the browser would handle the storage of the file in a temp directory.

@Jaifroid
Copy link
Member

OK, here I have to explain that the Chromium (Edge/Chrome) browser extension (and the UWP/PWA versions) operate in two modes:

The frist mode is the one you're currently using, called "jQuery" mode. This mode manipulates the DOM, and in this particular case it intervenes when a user clicks on a downloadable file type (PDF/EPUB), and adds some code to make it possible for the extension to trigger a download. However we can't change Response headers in this mode. We just tell the browser it is downloading a non-HTML file and let it do the rest. This mode is feature frozen in Kiwix JS, and only being bugfixed.

The second mode uses a Service Worker. In this case, the app simply executes whatever download code is defined in the ZIM, and doesn't intervene at all. It would be possible in theory to do what you ask in SW mode (see Expert Settings in Config) by changing the Response header to add the Content-Disposition header (the code would need to be added in service-worker.js). However, this is a "purist" mode, and the philosophy is not to add any customization that is not defined in the ZIM. We want this mode to be the default, probably from the next release (the former mode will still be supported, but not developed).

It would be possible for you to ask for this functionality to be supported in the ZIM's source code (I can tell you where to ask for that, or move this issue, if you tell me which ZIM you are using). But I don't think implementing this in the app would be supported here in Kiwix JS.

In Kiwix JS PWA/Electron/Windows, I am forced to intervene (in both modes) because I have to use UWP APIs, Electron APIs, etc., so there is more scope for customization there than in the browser extensions here.

@cjohnsto-nz
Copy link
Author

Gotcha. I'm not particularly familiar with the ZIM format. Would it be possible to add response headers, or something that would achieve the desired results, through post processing of the ZIM? I'm generating the ZIM, and would happily customize it to make it more convenient for our users, especially if it can be automated.

You are an absolute legend by the way :)

@cjohnsto-nz
Copy link
Author

cjohnsto-nz commented Sep 17, 2021

Mate, it opens inline natively in service worker mode. I didn't even think to try.

@Jaifroid
Copy link
Member

Mate, it opens inline natively in service worker mode. I didn't even think to try.

That's good to know! But I imagine it still creates duplicate version, right?

@cjohnsto-nz
Copy link
Author

Mate, it opens inline natively in service worker mode. I didn't even think to try.

That's good to know! But I imagine it still creates duplicate version, right?

I don't think so. I used voidtools Everything just to monitor the filesystem while I accessed the file. It looks like, if the browser opens inline, it creates a .tmp file in C:\Users*\AppData\Local\Temp, and destroys it when the Window is closed. Perfect for my usecase.

@Jaifroid
Copy link
Member

So I think we can close the issue here. @mossroy I think this demonstrates quite well why we need to implement #196, which also probably requires implementing #388 and #764, if we're not going to leave Firefox Extension users out in the cold!

@mossroy
Copy link
Contributor

mossroy commented Sep 17, 2021

I agree with you : we need #196 and #764 is certainly a way to allow (most) Firefox extension users to use SW mode

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