Avoid the getJavaScript API-call in PDFViewerApplication._initializeAutoPrint when "enableScripting" is set
#12765
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…zeAutoPrint` when "enableScripting" is set Rather than calling `getJavaScript` in the API and then ignoring the result, when "enableScripting" is set, it should be more efficient/faster to simply skip it altogether instead. Finally, the `setTimeout` call at the end of `PDFViewerApplication._initializeAutoPrint` is removed, since it doesn't seem necessary any more as far as I can tell.[1] Note that when this functionality was originally added, back in PR 2839, it seems that `pagesPromise` simply waited for the `getPage` calls of *all* pages to resolve. Today, on the other hand, the viewer fetches *and* renders the first page *before* doing the remaining `getPage` calls, and only afterwards is `pagesPromise` resolved. Hence it's not really clear why we now need to delay printing even further with a `setTimeout` call. --- [1] The patch was tested with the following documents: https://github.com/mozilla/pdf.js/blob/master/test/pdfs/bug1001080.pdf and https://github.com/mozilla/pdf.js/blob/master/test/pdfs/issue6106.pdf
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rather than calling
getJavaScriptin the API and then ignoring the result, when "enableScripting" is set, it should be more efficient/faster to simply skip it altogether instead.Finally, the
setTimeoutcall at the end ofPDFViewerApplication._initializeAutoPrintis removed, since it doesn't seem necessary any more as far as I can tell.[1]Note that when this functionality was originally added, back in PR #2839, it seems that
pagesPromisesimply waited for thegetPagecalls of all pages to resolve. Today, on the other hand, the viewer fetches and renders the first page before doing the remaininggetPagecalls, and only afterwards ispagesPromiseresolved. Hence it's not really clear why we now need to delay printing even further with asetTimeoutcall.[1] The patch was tested with the following documents: https://github.com/mozilla/pdf.js/blob/master/test/pdfs/bug1001080.pdf and https://github.com/mozilla/pdf.js/blob/master/test/pdfs/issue6106.pdf