With the current way that the `HandTool` is implemented, if someone would try to also add a Zoom tool (as issue 1260 asks for) that probably wouldn't work very well given that you'd then have two cursor tools which may not play nice together.
Hence this patch, which attempts to refactor things so that it should be simpler to add e.g. a Zoom tool as well (given that that issue is marked as "good-beginner-bug", and I'm not sure if that really applies considering the current state of the code).
Note that I personally have no interest in implementing a Zoom tool (similar to Adobe Reader) since I wouldn't use it, but I figured that it can't hurt to make this code a bit more future proof.
Also updates a `console.log` statement in the `gulp importl10n` command (since I forgot it in a previous patch, and it didn't seem necessary with a separate patch for it).
Also replaces `var` with `let` in code that's touched in the patch. Please note that this should be completely safe, for two separate reasons, since trying to access let in a scope where it's not defined is first of all a runtime error and second of all an ESLint error (thanks to the `no-undef` rule).
Refactors `Driver._cleanup` to return a `Promise` which is resolved once all opened documents have been destroyed.
This is then used in `Driver._nextTask` to ensure that we wait for everything to be cleaned up, such that the tests run sequentially.
Currently this method first uses a loop to build a temporary array to hold Promises, which are then resolved from a recursive helper function once the textContent is fetched for each page.
To me, this is unncessarily complicated, since we can do everything within one loop by simply chaining the asynchronous calls to retrieve the textContent. (Note that this guarantees that the textContent of the pages is still fetched sequentially.)
Also replaces `var` with `let` in the functions/methods that are touched in the patch. Please note that this should be completely safe, for two separate reasons, since trying to access `let` in a scope where it's not defined is first of all a runtime error and second of all an ESLint error (thanks to the `no-undef` rule).
This is a downstream change introduced in [1]. That mozilla bug is adding a new property to channel's loadinfo object (nsILoadInfo) that protocol handlers has to set on channels when originalURI on the result channel is set to a different URI than the channel has been created for.
Existence of the new property on nsILoadInfo depends on landing [1].
[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1319111
- the viewer was not loading in development mode on Safari, due to Safari
having crypto.webkitSubtle instead of crypto.subtle.
- the isCachingPossible check was amended to check for crypro.subtle
which is currently not in Safari but in Firefox and Chrome. This
essentially works around the issue by disabling caching for Safari
in development mode.
- maintainer sentiment: people who develop on Safari can get this speedup
once Safari drops prefix for SubtleCrypto.
- note: at time of writing Safari Version 10.1 (12603.1.30.0.34) has an
issue where caching can be enabled for PDF.js but must to be disabled for
worker, otherwise the two sides do not communicate.
- https://github.com/mozilla/pdf.js/pull/8387#issuecomment-299709961
- https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto/digest
For some reason, we're putting all kind of images *except* JPEG into the `imageCache` in `evaluator.js`.[1]
This means that in the PDF file in issue 8380, we'll keep sending the *same* two small images[2] to the main-thread and decoding them over and over. This is obviously hugely inefficient!
As can be seen from the discussion in the issue, the performance becomes *extremely* bad if the user has the addon "Adblock Plus" installed. However, even in a clean Firefox profile, the performance isn't that great.
This patch not only addresses the performance implications of the "Adblock Plus" addon together with that particular PDF file, but it *also* improves the rendering times considerably for *all* users.
Locally, with a clean profile, the rendering times are reduced from `~2000 ms` to `~500 ms` for my setup!
Obviously, the general structure of the PDF file and its operator sequence is still hugely inefficient, however I'd say that the performance with this patch is good enough to consider the issue (as it stands) resolved.[3]
Fixes 8380.
---
[1] Not technically true, since inline images are cached from `parser.js`, but whatever :-)
[2] The two JPEG images have dimensions 1x2, respectively 4x2.
[3] To make this even more efficient, a new state would have to be added to the `QueueOptimizer`. Given that PDF files this stupid fortunately aren't too common, I'm not convinced that it's worth doing.
This patch contains the following improvements:
- Only fetch the various document properties *once* per PDF file opened, and cache the result (in a frozen object).
- Always update the *entire* dialog at once, to prevent inconsistent UI state (issue 8371).
- Ensure that the dialog, and all its internal properties, are reset when `PDFViewerApplication.close` is called.
- Inline, and re-factor, the `getProperties` method in `open`, since that's the only call-site.
- Always overwrite the fileSize with the value obtained from `pdfDocument.getDownloadInfo`, to ensure that it's correct.
- ES6-ify the code that's touched in this patch.
Fixes 8371.
