-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Fixed basic.html and updated 'pdfjsnewwindow' #2855
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR.
@@ -3026,7 +3026,7 @@ function jsPDF(options) { | |||
"<style>html, body { padding: 0; margin: 0; } iframe { width: 100%; height: 100%; border: 0;} </style>" + | |||
'<body><iframe id="pdfViewer" src="' + | |||
pdfJsUrl + | |||
'?file=" width="500px" height="400px" />' + | |||
'?file=&downloadName=' + options.filename + '" width="500px" height="400px" />' + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The arguments seem a little messed up now...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only way so far that I can think of to pass the file name for download using pdf.js. Currently, it defaults to "ducment.pdf". The arguments in viewer.js seem always has only one element - the blob URL, so it's not useful for that matter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that's alright. It's just that the URL parameters now look like this
?file=&downloadName=filename.pdf
which looks a bit broken. Do you know what the "file=" part does? Seems to me like we can remove that part.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, ok. Didn't test it. Then everything is fine ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Just for the record, it passes the bloburl as the file name to pdf.js first, then replaces it with a physical pdf file name/path if exists. How the bloburl is inserted as the file name is not clear to me yet.
Restarting once again... hope it will work this time... |
Fixed basic.html to work with the new release;
Updated output('pdfjsnewwindow') to allow user-defined title and download file name