Check isEvalSupported, and test that eval is actually supported, before attempting to use the PostScriptCompiler (issue 5573)
#8909
Conversation
…before attempting to use the `PostScriptCompiler` (issue 5573) Currently `PDFFunction` is implemented (basically) like a class with only `static` methods. Since it's used directly in a number of different `src/core/` files, attempting to pass in `isEvalSupported` would result in code that's *very* messy, not to mention difficult to maintain (since *every* single `PDFFunction` method call would need to include a `isEvalSupported` argument). Rather than having to wait for a possible re-factoring of `PDFFunction` that would avoid the above problems by design, it probably makes sense to at least set `isEvalSupported` globally for `PDFFunction`. Please note that there's one caveat with this solution: If `PDFJS.getDocument` is used to open multiple files simultaneously, with *different* `PDFJS.isEvalSupported` values set before each call, then the last one will always win. However, that seems like enough of an edge-case that we shouldn't have to worry about it. Besides, since we'll also test that `eval` is actually supported, it should be fine. Fixes 5573.
Snuffleupagus
changed the title
isEvalSupported, and test that eval is actually supported, before attempting to use the PostScriptCompilerisEvalSupported, and test that eval is actually supported, before attempting to use the PostScriptCompiler (issue 5573)
|
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/150e7767b924fdc/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.215.176.217:8877/d24f22669906923/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/150e7767b924fdc/output.txt Total script time: 16.69 mins
|
From: Bot.io (Windows)SuccessFull output at http://54.215.176.217:8877/d24f22669906923/output.txt Total script time: 29.81 mins
|
|
/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/7f5c455b637092e/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/7f5c455b637092e/output.txt Total script time: 2.34 mins Published |
|
Looks good. Given the current state of that code, I agree that this is the nicest solution for now. |
|
Worker can handle multiple documents and handling isEvalSupported this way may cause confusion. When need to 1) make PDFFunction factory, which is constructed based on isEvalSupported passed to worker, 2) or, pass it similar to verbosity option. |
Sure, I'm currently looking into re-factoring |
…Supported Check `isEvalSupported`, and test that `eval` is actually supported, before attempting to use the `PostScriptCompiler` (issue 5573)
Currently
PDFFunctionis implemented (basically) like a class with onlystaticmethods. Since it's used directly in a number of differentsrc/core/files, attempting to pass inisEvalSupportedwould result in code that's very messy, not to mention difficult to maintain (since every singlePDFFunctionmethod call would need to include aisEvalSupportedargument).Rather than having to wait for a possible re-factoring of
PDFFunctionthat would avoid the above problems by design, it probably makes sense to at least setisEvalSupportedglobally forPDFFunction.Please note that there's one caveat with this solution: If
PDFJS.getDocumentis used to open multiple files simultaneously, with differentPDFJS.isEvalSupportedvalues set before each call, then the last one will always win.However, that seems like enough of an edge-case that we shouldn't have to worry about it. Besides, since we'll also test that
evalis actually supported, it should be fine.Fixes #5573.
Supersedes PR #8815 (since that one hasn't seen any activity for weeks, and based on the findings above I'm no longer entirely convinced that it would have been a simple beginner bug).
Edit: Slightly smaller diff with https://github.com/mozilla/pdf.js/pull/8909/files?w=1.