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

Upgrade viewer to PDF.js v1.6.210 #10

Merged
merged 10 commits into from
Mar 13, 2017
Merged

Conversation

tmcdos
Copy link
Contributor

@tmcdos tmcdos commented Mar 9, 2017

As requested, I upgraded PDF.js to the current stable version 1.6.210 and then reapplied the patches from the README. Some of them are no more relevant so I updated the README accordingly. There is a test deployment available on http://test.ivogelov.com/pdf/
There are some warnings in the browser console about undefined localization strings.

@moesjarraf
Copy link
Member

moesjarraf commented Mar 9, 2017

I have tested your changes with our angular-pdfjs-viewer demo. It looks good but a couple of things aren't working anymore. Since we do not have automatic tests we will have to manually tests that things aren't broken. Can you please verify/fix the following:

  1. Make sure the buttons are working (print for example throws an error Uncaught TypeError: Cannot set property 'onclick' of null.
  2. It seems to be no longer working in IE 11 (blank screen). Please test across browsers that it works. The old version did work in IE 11, also the live mozilla demo works there too.

Also, it would have been a lot clearer and easier for the reviewing process to make the patches on the actual source code of mozilla. I made a fork here. Could you add the patches onto the source code? Based on that we can generate the two files (pdf.js, pdf.worker.js) that this project depends on. We could then keep the fork easily up to date with the upstream of mozilla.

@tmcdos
Copy link
Contributor Author

tmcdos commented Mar 9, 2017

You were right, the PRINT button was not working because I have not updated "viewer.html" to reflect the latest changes in pdf.js. The LOAD button was not working because one of the patches was too restrictive (the one that disables change notifications for all file-type form elements). I have added one command to "build.sh" which will synchronize "viewer.html" with the latest version from pdf.js

Updated build script and removed patches
@tmcdos
Copy link
Contributor Author

tmcdos commented Mar 13, 2017

@moesjarraf I merged your changes.

@moesjarraf moesjarraf merged commit 61adf57 into legalthings:master Mar 13, 2017
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

Successfully merging this pull request may close these issues.

2 participants