As mentioned in https://github.com/mozilla/pdf.js/wiki/Frequently-Asked-Questions#faq-support, PDF.js version `2.6.347` is the last release with IE 11/Edge support.
Hence we should now be able to reduce unnecessary duplication in the default viewer image resources, note the files in the `web/images/` folder with a `-dark` suffix, by using only *one* SVG-image for each icon and letting the `background-color` depend on the CSS theme instead.
For the `gulp mozcentral` build-target, the resulting `web/images/` folder is reduced from `43 997` to `28 566` bytes (~35 percent).
*Please note:* I don't really know if this implementation is necessarily the *best* solution, but it seems to work well enough in e.g. Firefox Nightly and Google Chrome Beta as far as my testing goes.
In addition to the existing /Root and /Pages validation, also check that the /Pages-entry actually is a dictionary and that it has a valid /Count-entry.
This way we can avoid picking a trailer candidate which e.g. the `Catalog.numPages` getter will just end up rejecting, thus breaking PDF document loading completely.
Given that we already include the "Content-Disposition"-header filename, when it exists, it shouldn't hurt to also include the information from the "Content-Length"-header.
For PDF documents opened via a URL, which should be a very common way for the PDF.js library to be used, this will[1] thus provide a way of getting the PDF filesize without having to wait for the `getDownloadInfo`-promise to resolve[2].
With these API improvements, we can also simplify the filesize handling in the `PDFDocumentProperties` class.
---
[1] Assuming that the server is correctly configured, of course.
[2] Since that's not *guaranteed* to happen in general, with e.g. `disableAutoFetch = true` set.
* quickjs-eval.js has been generated using https://github.com/mozilla/pdf.js.quickjs/
* lazy load of sandbox code
* Rewrite tests to use the sandbox
* Add a task `watch-sandbox` which update bundle pdf.sandbox.js on change in the sandbox code
Given that it's generally faster to call *one* function and have it loop through an object, rather than looping through an object and calling a function for every iteration, this patch will reduce the total time spent in `PDFViewerApplication._readPreferences` ever so slightly.
Also, over time we've been adding more and more preferences, rather than removing them, so using the new `AppOptions.setAll` method should be generally beneficial as well.
While the effect of these changes is quite small, it does reduces the time it takes for the preferences to be fully initialized. Given the amount of asynchronous code during viewer initialization, every bit of time that we can save should thus help.
Especially considering the recently added `viewerCssTheme` preference, which needs to be read very early to reduce the risk of the viewer UI "flashing" visibly as the theme changes, I figured that a couple of small patches reducing the time spend reading preferences cannot hurt.
Given that only two debugging hash parameters (i.e. `disableWorker` and `pdfBug`) will make this method asynchronous, we can avoid what's most of the time is an unnecessary `Promise.all` invocation.
While this does work pretty well in my quick testing, it's *very much* a hack since as far as I can tell there's no support in the CSS specification for using e.g. a CSS variable to override a `@media (prefers-color-scheme: dark) {...}` block.
The solution implemented here is thus to *edit* the viewer CSS, by either removing the entire `@media ...` block in light-mode or by ensuring that its rules become *unconditionally* applied in dark-mode.
To simplify the overall implementation, since all of this does seem like somewhat of an edge-case, the `viewerCssTheme` preference will *only* be read during viewer initialization. (Similar to many other existing preferences, a reload is thus required when changing it.)
Originally the default preferences were defined in a JSON-file checked into the repository, which was loaded using SystemJS in development mode.
Over the years a number of changes have been made to this code, most notably:
- The preferences JSON-file is now generated automatically, during building, from the `AppOptions` abstraction.
- All SystemJS usage has been removed from the development viewer.
Hence the default preferences are now available *synchronously* even in the development viewer, and it's thus no longer necessary to defer to the microtask queue (since `getDefaultPreferences` is async) just to get the default preferences.
While the effect of these changes is quite small, it *does* reduces the time it takes for the preferences to be fully initialized. Given the amount of asynchronous code during viewer initialization, every bit of time that we can save should thus help.
- Add support for logical assignment operators, i.e. `&&=`, `||=`, and `??=`, with a Babel-plugin. Given that these required incrementing the ECMAScript version in the ESLint and Acorn configurations, and that platform/browser support is still fairly limited, always transpiling them seems appropriate for now.
- Cache the `hasJSActions` promise in the API, similar to the existing `getAnnotations` caching. With this implemented, the lookup should now be cheap enough that it can be called unconditionally in the viewer.
- Slightly improve cleanup of resources when destroying the `WorkerTransport`.
- Remove the `annotationStorage`-property from the `PDFPageView` constructor, since it's not necessary and also brings it more inline with the `BaseViewer`.
- Update the `BaseViewer.createAnnotationLayerBuilder` method to actaually agree with the `IPDFAnnotationLayerFactory` interface.[1]
- Slightly tweak a couple of JSDoc comments.
---
[1] We probably ought to re-factor both the `IPDFTextLayerFactory` and `IPDFAnnotationLayerFactory` interfaces to take parameter objects instead, since especially the `IPDFAnnotationLayerFactory` one is becoming quite unwieldy. Given that that would likely be a breaking change for any custom viewer-components implementation, this probably requires careful deprecation.
*Note that I wasn't able to reproduce the issue in Firefox, but only in Chromium-browsers.*
The bug, and it's feels almost trivial once you've found it, is that we're not passing the `transform` parameter as intended to `PDFPageProxy.render` when drawing thumbnails on HiDPI displays. Instead the canvas context is, for reasons that I don't even pretent to understand, *manually* scaled in `PDFThumbnailView._getPageDrawContext`, which thus doesn't guarantee that the `baseTransform` property on the `CanvasGraphics`-instances becomes correct.
The solution is really simple though, just handle the `transform` the same way in `PDFThumbnailView.draw` as in `PDFPageView.paintOnCanvas` and things should just work.
*This is a pre-existing issue that I noticed while working on PR 12613, and fixing this also brings the thumbnail code inline with the page code.*
Given the intermittent nature of all of this, it's somewhat difficult to reproduce it consistently; however the following steps should at least provide an outline:
1. Open the sidebar, and the thumbnailView, and start scrolling around.
2. *Quickly* close the sidebar, so that all thumbnails won't have time to finish rendering.
3. Either wait for the cleanup-timeout to occur, or simply run `PDFViewerApplication.cleanup()` in the console.
What *intermittently* happens here is that `WorkerTransport.startCleanup` rejects, and consequently that cleanup doesn't complete as intended, since some of the thumbnails are left in a *pending* renderingState[1].
Fixing this is simple though, and only requires updating `PDFThumbnailViewer.cleanup` along the lines of `BaseViewer.cleanup`.
---
[1] Keep in mind that thumbnails will *only* render when the thumbnailView is visible, to reduce resource usage.
This patch will help reduce memory usage, especially for longer documents, when the user scrolls around in the thumbnailView (in the sidebar).
Note how the `PDFPageProxy.cleanup` method will, assuming it's safe to do so, release main-thread resources associated with the page. These include things such as e.g. image data (which can be arbitrarily large), and also the operatorList (which can also be quite large).
Hence when pages are evicted from the `PDFPageViewBuffer`, on the `BaseViewer`-instance, the `PDFPageView.destroy` method is invoked which will (among other things) call `PDFPageProxy.cleanup` in the API.
However, looking at the `PDFThumbnailViewer`/`PDFThumbnailView` classes you'll notice that there's no attempt to ever call `PDFPageProxy.cleanup`, which implies that in certain circumstances we'll essentially keep all resources allocated permanently on the `PDFPageProxy`-instances in the API.
In particular, this happens when the users opens the sidebar and starts scrolling around in the thumbnails. Generally speaking you obviously need to keep all thumbnail *images* around, since otherwise the thumbnailView is useless, but there's still room for improvement here.
Please note that the case where a *rendered page* is used to create the thumbnail is (obviously) completely unaffected by the issues described above, and this rather only applies to thumbnails being explicitly rendered by the `PDFThumbnailView.draw` method.
For the latter case, we can fix these issues simply by calling `PDFPageProxy.cleanup` once rendering has finished. To prevent *accidentally* pulling the rug out from under `PDFPageViewBuffer` in the viewer, which expects data to be available, this required adding a couple of new methods[1] to enable checking that it's indeed safe to call `PDFPageProxy.cleanup` from the `PDFThumbnailView.draw` method.
It's really quite fascinating that no one has noticed this issue before, since it's been around since basically "forever".
---
[1] While it should be *very* rare for `PDFThumbnailView.draw` to be called for a pageView that's also in the `PDFPageViewBuffer`, given that pages are rendered before thumbnails and that the *rendered page* is used to create the thumbnail, it can still happen since rendering is asynchronous.
Furthermore, it's also possible for `PDFThumbnailView.setImage` to be disabled, in which case checking the `PDFPageViewBuffer` for active pageViews *really* matters.
