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

support urlencode contentDispositionFilename #10381

Closed
wants to merge 2 commits into from
Closed

support urlencode contentDispositionFilename #10381

wants to merge 2 commits into from

Conversation

wangsongyan
Copy link
Contributor

No description provided.

@@ -1074,6 +1074,9 @@ let PDFViewerApplication = {
({ info, metadata, contentDispositionFilename, }) => {
this.documentInfo = info;
this.metadata = metadata;
if(contentDispositionFilename && contentDispositionFilename.includes('%')){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not look like the correct place to fix it. Instead, it should be fixed in the function where the content disposition filename is fetched, most likely extractFilenameFromHeader. Moreover, a unit test needs to be added to

describe('extractFilenameFromHeader', function() {
to make sure it works. Finally, please elaborate a bit about what the exact problem is in the pull request description.

@timvandermeij
Copy link
Contributor

Closing since this cannot be merged in the current state and no response was given. If you do believe something is wrong in PDF.js, please open an issue so we can discuss it and find out what the best fix would be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants