Refactor the DownloadManager initialization in GENERIC/CHROME builds again (PR 8203 follow-up)
#8300
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
…uilds again (PR 8203 follow-up) In the first commit in PR 8203, I changed how the `DownloadManager` was included/initialized in `GENERIC`/`CHROME` builds. The change was prompted by the fact that you cannot have conditional `import`s with ES6 modules, and I wanted to avoid bundling the general `DownloadManager` into the various Firefox specific build targets. What I completely missed though, is that the new code meant that `download_manager.js` will now be pulling in the *entire* viewer (through `app.js`). This is a *really* stupid mistake on my part, since it causes the `dist/build/pdf_viewer.js` used with the viewer components to now include basically the entire default viewer. The simplest solution that I could come up with, is to add a `genericcom.js` file (similar to the `firefoxcom.js`/`chromecom.js` files) which will be responsible for importing/initializing the `DownloadManager`.
From: Bot.io (Linux)ReceivedCommand cmd_preview from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.21.233.14:8877/af08fd5825fd3ff/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/af08fd5825fd3ff/output.txt Total script time: 3.02 mins Published |
yurydelendik
approved these changes
Apr 17, 2017
movsb
pushed a commit
to movsb/pdf.js
that referenced
this pull request
Jul 14, 2018
…nager-creation Refactor the `DownloadManager` initialization in `GENERIC`/`CHROME` builds again (PR 8203 follow-up)
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.
In the first commit in PR #8203, I changed how the
DownloadManagerwas included/initialized inGENERIC/CHROMEbuilds.The change was prompted by the fact that you cannot have conditional
imports with ES6 modules, and I wanted to avoid bundling the generalDownloadManagerinto the various Firefox specific build targets.What I completely missed though, is that the new code meant that
download_manager.jswill now be pulling in the entire viewer (throughapp.js).This is a really stupid mistake on my part, since it causes the special
pdf_viewer.jsfile used with the viewer components to now include basically the entire default viewer.The simplest solution that I could come up with, is to add a
genericcom.jsfile (similar to thefirefoxcom.js/chromecom.jsfiles) which will be responsible for importing/initializing theDownloadManager.This should fix the remaining warnings in issue #8292, and also restores the viewer components specific
pdf_viewer.jsfile to its intended scope/size.Edit: Adding the
genericcom.jsfile might seem a bit unnecessary at the moment, however I've got an idea for refactoring ofhttps://github.com/mozilla/pdf.js/blob/master/web/preferences.jsinto a proper class and that work would leverage the newgenericcom.jsfile.