Skip to content

Conversation

@danxuliu
Copy link
Member

Fixes #52

This pull request updates PDF.js to 1.9.426 (the latest stable version at the time of this writing).

The PDF.js code in vendor had no patches, so the files were simply replaced with the new ones (although some unneeded files like the example PDF were removed); templates/viewer.php and js/workersrc.js were updated with the changes needed for the new version, but respecting their customizations.

Besides that, due to the changes from PDF.js 1.4.20 to 1.9.426 opening a local file failed with a Content Security Policy violation; instead of fixing that the Open file button was simply hidden, as being able to open a local PDF file provided no value (at least, in most cases; it was probably a must have feature for someone out there).

Finally, the Current view (copy or open in new window) button does not seem to work, but it did not either in PDF.js 1.4.20. To be honest I do not know what is the expected behaviour of that button, so I left it as is.

The example PDF ("compressed.tracemonkey-pldi-09.pdf") is not needed and
thus was not included; "templates/viewer.php" acts as "viewer.html", so
the later was not included either.

On the other hand, source maps were kept so they could be used in case
of need.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
In PDF.js 1.9.426 it is no longer possible to simply set the custom
values for "PDFJS" and "PDFViewerApplication" when "js/workersrc.js" is
parsed. When "PDFViewerApplication.initialize" is executed it overwrites
the value of "PDFJS.workerSrc", so the custom initialization has to be
executed after that. As "PDFViewerApplication" does not provide any
hookable point for custom initialization in its initialization routine a
dirty hack has to be used.

When "vendor/pdfjs/web/viewer.js" is parsed at the end it calls
"PDFViewerApplication.initialize" (either directly or as a
"DOMContentLoaded" event handler), and "PDFViewerApplication.initialize"
sends an asynchronous XHR request to initialize the locales, which
causes the execution flow to continue with the next script or the next
"DOMContentLoaded" handler. Thanks to this
"initializeCustomPDFViewerApplication" can be executed at that point by
parsing "js/workersrc.js" after "vendor/pdfjs/web/viewer.js" and either
calling it directly or adding it as a "DOMContentLoaded" event handler
(using the same conditions as in "vendor/pdfjs/web/viewer.js").

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
The "viewer.php" acts as the "viewer.html" file included in the PDF.js
package. This commit updates "viewer.php" with the changes to
"viewer.html" from PDF.js 1.4.20 to 1.9.426 while respecting the
customized parts of "viewer.php" (like the close button or the "nonce"
attribute).

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Due to the changes from PDF.js 1.4.20 to 1.9.426 now trying to open a
different PDF file fails with a Content Security Policy violation.
Instead of fixing that the "Open file" button was disabled, as being
able to open a different PDF file from the local file system to view it
instead of the PDF file in the server did not provide much value.

The button was disabled by setting the "hidden" attribute to "true"
instead of with the CSS class for consistency with how it is done in
"vendor/pdfjs/web/viewer.js" when the browser does not support any of
the APIs needed to open the local file.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Tested and works nicely. Sharing page as well as integrated 👍

@MorrisJobke MorrisJobke merged commit 80cb8ca into master Feb 24, 2018
@MorrisJobke MorrisJobke deleted the update-pdf.js-to-1.9.426 branch February 24, 2018 12:02
@MorrisJobke
Copy link
Member

@danxuliu @rullzer I would backport this ti stable13 at least and then people can use this (especially for the IE printing problems & stuff like that)

@danxuliu
Copy link
Member Author

I would backport this ti stable13 at least

As the backport to stable12 was trivial and it improves the PDF support I did that too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PDF.js needs updating.

3 participants