search

legalthings / pdf.js-viewer

Compiled version of pdf.js viewer, modified to run embedded
Apache License 2.0
45 stars 55 forks source link

Upgrade viewer to PDF.js v1.6.210 #10

Closed tmcdos closed 7 years ago

tmcdos commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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

tmcdos commented 7 years ago

@moesjarraf I merged your changes.