Remove the disableCanvasToImageConversion functionality from PDFThumbnailView (PR 7029 follow-up)
#13357
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
…umbnailView` (PR 7029 follow-up) This functionality was originally implemented in PR 7029; however it's not, nor has it ever been, used as far as I can tell.[1] Note in particular that the default viewer does not expose either a preference or even an option with which `disableCanvasToImageConversion` can be toggled, and source-code modification is thus required. Furthermore, note also that we have multiple other instances of `canvas`-data accesses in both the `src/display/canvas.js` and `src/display/text_layer.js` files. If any of those are blocked, by e.g. browser settings, there will be outright rendering bugs and non-working thumbnails thus seem like a very small issue in the grand scheme of things; hence why I'm suggesting that we remove the unused `disableCanvasToImageConversion` functionality. --- [1] For the Tor use-case mentioned in issue 7026, I *believe* that the solution was to white-list `canvas`-data accesses for its built-in PDF Viewer.
|
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @timvandermeij received. Current queue size: 0 Live output at: http://54.67.70.0:8877/0044be66578f391/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/0044be66578f391/output.txt Total script time: 4.49 mins Published |
timvandermeij
approved these changes
May 9, 2021
|
Looks like a good plan to me. 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.
This functionality was originally implemented in PR #7029; however it's not, nor has it ever been, used as far as I can tell.[1]
Note in particular that the default viewer does not expose either a preference or even an option with which
disableCanvasToImageConversioncan be toggled, and source-code modification is thus required.Furthermore, note also that we have multiple other instances of
canvas-data accesses in both thesrc/display/canvas.jsandsrc/display/text_layer.jsfiles. If any of those are blocked, by e.g. browser settings, there will be outright rendering bugs and non-working thumbnails thus seem like a very small issue in the grand scheme of things; hence why I'm suggesting that we remove the unuseddisableCanvasToImageConversionfunctionality.[1] For the Tor use-case mentioned in issue #7026, I believe that the solution was to white-list
canvas-data accesses for its built-in PDF Viewer.