download icon indicating copy to clipboard operation
download copied to clipboard

Cannot open PDF file after download

Open taiducnguyen opened this issue 7 years ago • 17 comments

Please help to check, this is my code:

  var fileUrl = form.vals().download;
  var fileName = fileUrl ? fileUrl.replace(/^.*[\\\/]/, '') : '';
  download(fileUrl, fileName, "application/pdf");

After download, that file can not open: image

taiducnguyen avatar Nov 09 '17 02:11 taiducnguyen

If you just do: download(fileUrl); it works

Adding a filename in 2nd argument makes the script to create a txt file containing text string of first argument, which is very strange. It would have been clearly better that the method receive an object with filename, mime, text, url, etc properties instead of 3 coma-separated arguments.

realjck avatar Dec 21 '17 15:12 realjck

+1

BenMGilman avatar Apr 12 '18 15:04 BenMGilman

Is there a better solution for this available? My pdf download url does not have an extension. The good browsers are smart enough to figure out that its a pdf based on the response header, but IE(11) doesnt add the extension.

Jelle-vz avatar Jul 18 '18 11:07 Jelle-vz

Same problem here @jckleinbourg

Did you solve the problem?

jonathanpmartins avatar Oct 10 '18 21:10 jonathanpmartins

For this case, you can fix (hack) the last version v4.21 by changing:

line 31: url = !strFileName && !strMimeType && payload, to: url = strFileName && !strMimeType && payload,

and disabling line 48: // fileName = url.split("/").pop().split("?")[0];

Thus I am not sure if the script still works for other cases (EDIT: actually when doing this, it does NOT extract filename from url anymore when passing only url), but it works OK now when passing url + filename. By reading the source code, I think that it would need a complete rework by using object data instead of argument, like this:

download({
    url:...,
    data:...,
    strFileName:...,
    etc...
});

The problem here is that the first argument can be in some cases a url and in some other cases data, it's typically bad practice.

Therefore, doing a "clean" v5 version would break the code for those who use the older versions in their project (or it would need an hybrid version that checks if the first argument is an expected object).

realjck avatar Oct 11 '18 09:10 realjck

I think this line is intended for checking whether the parameter is url, and he restricts the number of args to only one, it doesn't support download url with custom filename/mime type or it only work like a data saver https://github.com/rndme/download/blob/09d6492f47ef18feca39c3d748960dce44f93a89/download.js#L31 According to this, I modified line 31 and 48, here is the file, and also fix the arguments.callee error under strict mode.

At least, it works fine for me, hope it will help.

dogbutcat avatar Oct 13 '18 00:10 dogbutcat

I think this line is intended for checking whether the parameter is url, and he restricts the number of args to only one, it doesn't support download url with custom filename/mime type or it only work like a data saver download/download.js

Line 31 in 09d6492

url = !strFileName && !strMimeType && payload,

According to this, I modified line 31 and 48, here is the file, and also fix the arguments.callee error under strict mode. At least, it works fine for me, hope it will help.

Hi dogbutcat I hate to say this, but your version doesn't work at all for me, in all cases (tested on Chrome last version).

When doing: download("folder/some_file.pdf");

it saves with name "download", and when renaming to "file.pdf", it gives an error when openning file.

And when doing: download("folder/some_file.pdf", "new_name.pdf");

it saves with name "new_name.pdf", and still gives an error when openning file.

Are you sure that you commited and published your last fixed version?

Also I saw that you removed the bubbling stopEventPropagation() methods at some point in the code, is there a reason for this?

In all cases thanks for your time on this.

EDIT: If the new version works for you, can you please give an exemple to see how you call the function in your case?

realjck avatar Oct 13 '18 01:10 realjck

@jckleinbourg As you see the code, I only check the parameter with http(s):// prefix. For your case, is that a relative path? If so, sorry for that, it still doesn't support as I haven't meet that need.

Ps: I don't remove the bubbling😂, I only moved the function out to avoid using arguments.callee

dogbutcat avatar Oct 13 '18 01:10 dogbutcat

@jckleinbourg As you see the code, I only check the parameter with http(s):// prefix. For your case, is that a relative path? If so, sorry for that, it still doesn't support as I haven't meet that need.

Ah!! ok. That explains why.

realjck avatar Oct 13 '18 01:10 realjck

@jckleinbourg Yep! IMO it's better to work with absolute path as this function is used for download file, parsing address maybe not suitable to work inside the func & should work outside and passing to this to download.

Ps: oh, it's my fault, in original version, it support with only url param, I'll change the code. Many Thanks for inspiring!

dogbutcat avatar Oct 13 '18 01:10 dogbutcat

@jckleinbourg Yep! IMO it's better to work with absolute path as this function is used for download file, parsing address maybe not suitable to work inside the func & should work outside and passing to this to download.

I see what you mean, but the last versions were working regardless of absolute or relative paths. In anyway your last version works OK when using absolute paths (altough it doesn't extract the name from the url when specifing only url (the original version was doing this). But it works perfect when specifing absolute url + filename (I just figured out that my fix was working only with relative url (that is how I use it in my project), and was not working with absolute paths!)

So what I think, is that is a true headache to know if the first argument is a blob, an absolute url, or a relative path, and that it would be a good idea to make a new project from this with a clean object as argument to call the method instead of these comma separated undefined arguments (as I explain above).

And I think that this script works very well and is useful apart from this issue.

EDIT: what about having removed the stopPropagationEvent methods? Is it for your porject, or does it fix something else?

realjck avatar Oct 13 '18 02:10 realjck

@jckleinbourg I modified a new version here, removed the url match and revert back to original one, because the first param means to be data string or something else working with second or third param saving to a file, eg. I want to save a string like https://www.example.com/file.txt to filename test.pdf(it is wired indeed but working with original one).

So I toke your advice above, introducing a new way calling like(no bind support)

/**
 * @typedef {Object} RequestObject
 * @property {String} url - file url to download
 * @property {String} data - raw data to save
 * @property {String=} filename - custom filename to save
 * @property {String=} mimetype - custom file type
 */
download(obj:RequestObject);

My project work fine with such change no matter what kind of path. How about yours?

For the stopPropagationEvent, it is there before I fork. https://github.com/rndme/download/blob/09d6492f47ef18feca39c3d748960dce44f93a89/download.js#L114-L119 The anchor is append to document body, if the body has click event listener, it will also be triggered when execute anchor.click(). I don't know if is it necessary to append. I used to createElement and just dispatch event directly also working.

dogbutcat avatar Oct 15 '18 01:10 dogbutcat

Hello,

thanks for this library.

I do have the issue:

  • django server returns a fileresponse with mime type octet-stream
  • i used download to save the blob

I work with firefox

IF i use download(blob), the file is saved correctly, but not with the PDF extension

IF I use download(blob, 'file.pdf') the file type is shown as html, the file is corrupted IF I use download(blob, 'file.pdf', 'application/pdf') the file is corrupted

Will let you know if i find a way Trying to find a solution

acourdavault avatar Oct 19 '18 18:10 acourdavault

+1 large pdf file don't download

amihanya avatar Nov 05 '18 22:11 amihanya

+1 same problem, even with small 1 page pdf

tquiroga avatar Jul 20 '19 09:07 tquiroga

@realjck download(fileURL) is working BUT when I open the file it says 'Failed to open the file'

avillarubia-stationfive avatar May 21 '21 06:05 avillarubia-stationfive

+1 on this from me as well, really not ideal that specifying a url and name to rename to causes the download to be corrupted.

kevin-lindsay-1 avatar Sep 08 '21 18:09 kevin-lindsay-1