Remove the disableCreateObjectURL option from web/app_options.js
#12191
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
Prior to PR 11601, the `disableCreateObjectURL` option was present on `getDocument` in the API, since it was (potentially) used when decoding JPEG images natively in the browser. Hence setting this option, which was done automatically using compatibility-code, were in some browsers necessary in order for e.g. JPEG images to be correctly rendered. The downside of the `disableCreateObjectURL` option is that memory usage increases significantly, since we're forced to build and use `data:` URIs (rather than `blob:` URLs). However, at this point in time the `disableCreateObjectURL` option is only necessary for *some* (non-essential) functionality in the default viewer; in particular: - The openfile functionality, used only when manually opening a new file in the default viewer. - The download functionality, used when downloading either the PDF document itself or its attached files (if such exists). - The print functionality, in the generic `PDFPrintService` implementation. Hence neither the general PDF.js library, nor the *basic* functionality of the default viewer, depends on the `disableCreateObjectURL` option any more; which is why I'm thus proposing that we remove the option since using it is a performance footgun. *Please note:* To not outright break currently "supported" browsers, which lack proper `URL.createObjectURL` support, this patch purposely keeps the compatibility-code to explicitly disable `URL.createObjectURL` usage *only* for browsers which are known to not work correctly.[1] While it's certainly possible that there's additional, likely older, browsers with broken `URL.createObjectURL` support, the last time that these types of problems were reported was over *three* years ago.[2] Hence in the *very* unlikely event that additional problems occur, as a result of these changes, we can either add a new case in the compatibility-code or simply declare the affected browser as unsupported. --- [1] Which are IE11 (see issue 3977), and Google Chrome on iOS (see PR 8081). [2] Given that `URL.createObjectURL` is used by default, you'd really expect more reports if these problems were widespread.
|
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/0a9e84f5094aaf4/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/0a9e84f5094aaf4/output.txt Total script time: 3.34 mins Published |
timvandermeij
approved these changes
Aug 11, 2020
|
I agree with the points that you mentioned. Thanks! |
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.
Prior to PR #11601, the
disableCreateObjectURLoption was present ongetDocumentin the API, since it was (potentially) used when decoding JPEG images natively in the browser. Hence setting this option, which was done automatically using compatibility-code, were in some browsers necessary in order for e.g. JPEG images to be correctly rendered.The downside of the
disableCreateObjectURLoption is that memory usage increases significantly, since we're forced to build and usedata:URIs (rather thanblob:URLs).However, at this point in time the
disableCreateObjectURLoption is only necessary for some (non-essential) functionality in the default viewer; in particular:PDFPrintServiceimplementation.Hence neither the general PDF.js library, nor the basic functionality of the default viewer, depends on the
disableCreateObjectURLoption any more; which is why I'm thus proposing that we remove the option since using it is a performance footgun.Please note: To not outright break currently "supported" browsers, which lack proper
URL.createObjectURLsupport, this patch purposely keeps the compatibility-code to explicitly disableURL.createObjectURLusage only for browsers which are known to not work correctly.[1]While it's certainly possible that there's additional, likely older, browsers with broken
URL.createObjectURLsupport, the last time that these types of problems were reported was over three years ago.[2]Hence in the very unlikely event that additional problems occur, as a result of these changes, we can either add a new case in the compatibility-code or simply declare the affected browser as unsupported.
[1] Which are IE11 (see issue #3977), and Google Chrome on iOS (see PR #8081).
[2] Given that
URL.createObjectURLis used by default, you'd really expect more reports if these problems were widespread.