-
Notifications
You must be signed in to change notification settings - Fork 9.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Re-factor the PDFScriptingManager
-class for the viewer-components
#16687
Conversation
/botio unittest |
adcaadf
to
2a9ea07
Compare
2a9ea07
to
df54307
Compare
/botio-linux preview |
/botio unittest |
Currently this class contains a few "special" code-paths for the COMPONENTS build-target, which normally wouldn't be a problem. However, in this particular case that means accessing code that we don't want to include unconditionally in all builds. This is currently implemented using build-time `require`-calls which we nowadays want to avoid, and we should strive to remove all such cases from the code-base. (Generally speaking `import` is the future, and build-tools may not always play well with a mix of both formats.) We can easily improve things here by using sub-classing for the COMPONENTS build-target, and then use the ability to re-name when exporting (to avoid breaking existing code).
Having a `require` in this file has never made sense in e.g. the Firefox PDF Viewer and shouldn't really be necessary. Possibly the idea was to facilitate some kind of third-party bundling, however the *built* `pdf.js` file has always exposed the API-contents globally.
df54307
to
bad4bff
Compare
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/b5c60d57fee8135/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/b5c60d57fee8135/output.txt Total script time: 1.32 mins Published |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/35acbca57144e78/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.193.163.58:8877/15469477dbf705d/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/35acbca57144e78/output.txt Total script time: 25.00 mins
Image differences available at: http://54.241.84.105:8877/35acbca57144e78/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/15469477dbf705d/output.txt Total script time: 37.25 mins
Image differences available at: http://54.193.163.58:8877/15469477dbf705d/reftest-analyzer.html#web=eq.log |
Looks good to me! |
Currently this class contains a few "special" code-paths for the COMPONENTS build-target, which normally wouldn't be a problem. However, in this particular case that means accessing code that we don't want to include unconditionally in all builds.
This is currently implemented using build-time
require
-calls which we nowadays want to avoid, and we should strive to remove all such cases from the code-base. (Generally speakingimport
is the future, and build-tools may not always play well with a mix of both formats.)We can easily improve things here by using sub-classing for the COMPONENTS build-target, and then use the ability to re-name when exporting (to avoid breaking existing code).