*This patch is simple enough that I almost feel like I'm overlooking some trivial reason why this would be a bad idea.*
Note how in `{BaseViewer, PDFThumbnailViewer}.setDocument` we're always getting the *first* `pdfPage` in order to initialize all pages/thumbnails.
However, once that's done the first `pdfPage` is simply ignored and rendering of the first page thus requires calling `PDFDocumentProxy.getPage` yet again. (And in the `BaseViewer` case, it's even done once more after `onePageRenderedCapability` is resolved.)
All in all, I cannot see why we cannot just immediately set the first `pdfPage` and thus avoid an early round-trip to the API in the `_ensurePdfPageLoaded` method before rendering can begin.
Obviously the `_pagesRequests` functionality is *mainly* used when `disableAutoFetch` is set, but it will also be used during ranged/streamed loading of documents.
However, the `_pagesRequests` property is currently an Array which seems a bit strange:
- Arrays are zero-indexed, but the first element will never actually be set in the code.
- The `_pagesRequests` Array is never cleared, unless a new document is loaded, and once the `PDFDocumentProxy.getPage` call has resolved/rejected the element is just replaced by `null`.
- Unless the document is browsed *in order* the resulting `_pagesRequests` Array can also be arbitrarily sparse.
All in all, I don't believe that an Array is an appropriate data structure to use for this purpose.
If, as PR 10368 suggests, more parameters should be added to `getViewport` I think that it would be a mistake to not change the signature *first* to avoid needlessly unwieldy call-sites.
To not break any existing code and third-party use-cases, this is obviously implemented with a deprecation warning *and* with a working fallback[1] for the old method signature.
---
[1] This is limited to `GENERIC` builds, which should be sufficient.
The code responsible for highlighting/scrolling thumbnails is quite old, and parts of it hasn't really changed much over time.
In particular, the `scrollThumbnailIntoView` method is currently using `document.querySelector` in order to find both the new/current thumbnail element. This seems totally unnessary, since we can easily keep track of the current thumbnail (similar to the "regular" viewer) and thus avoid unnecessary DOM look-ups.
Furthermore, note how the `PDFThumbnailView` constructor is currently highlighting the *first* thumbnail. This is yet another leftover from older viewer code, which we can now remove and instead take care of in `PDFThumbnailViewer.setDocument`.
Given that `PDFThumbnailView` does not, nor should it, have any concept of which thumbnail is currently selected, this change also improves the general structure of this code a tiny bit.
If the sidebar is resized such that the thumbnails are displayed in multiple columns, then scrolling the currently active thumbnail into view doesn't work correctly in some cases.
The reason is that the code in `PDFThumbnailViewer.scrollThumbnailIntoView` implicitly assumes that the thumbnails will be present in just *one* column. Since that may no longer be the case, it's not sufficient to simply check if the thumbnail is visible. Instead we must explicitly check that *all*, i.e. 100 percent, of the thumbnail is already visible, and otherwise scroll it into view.
This changes both `PDFViewer` and `PDFThumbnailViewer` to return early in the `pagesRotation` setters if the rotation doesn't change.
It also fixes an existing issue, in `PDFViewer`, that would cause errors if the rotation changes *before* the scale has been set to a non-default value.
Finally, in preparation for subsequent patches, it also refactors the rotation code in `web/app.js` to update the thumbnails and trigger rendering with the new `rotationchanging` event.
Currently a number of these properties do not work correctly if set *before* calling `setDocument`; please refer to the discussion starting in https://github.com/mozilla/pdf.js/pull/8539#issuecomment-309706629.
Rather than trying to have *some* of these methods working, but not others, it seems much more consistent to simply always require that `setDocument` has been called.
This patch intends to simplify future ES6 refactoring of the thumbnail code, since the current code isn't going to work well together with proper `Class`es.
Please see http://eslint.org/docs/rules/object-shorthand.
For the most part, these changes are of the search-and-replace kind, and the previously enabled `no-undef` rule should complement the tests in helping ensure that no stupid errors crept into to the patch.
Note that as discussed on IRC, this makes the viewer slightly slower to load *only* in `gulp server` mode, however the difference seem slight enough that I think it will be fine.
This patch implements the page label functionality in a similar way as Adobe Reader.
For documents with page labels, if a non-existent page label is entered we'll try to fallback to the page number instead.
The patch also includes a preference (`disablePageLabels`), to make it easy to opt-out of using page labels if the user/implementor so wishes.
The way that `get/set currentPageLabel` is implemented in `PDFViewer`, is as wrappers for the corresponding `get/set currentPageNumber` functions, since that seemed like the cleanest solution.
The page labels are purposely *only* added to the page controls in the viewer UI, and not stored in e.g. the `ViewHistory`. Since doing so would mean adding unnecessary code complexity, without any real added value, and would also mean delaying the inital loading of PDF documents.
Note that this patch will ignore page labels if they are identical to standard page numbering, since in this case displaying the page labels adds no value (but only UI noise). The reason for handling this case specially, is that in practice a surprising number of PDF files include "pointless" page labels.
[PDFThumbnailView] Re-factor the `canvas` to `image` conversion such that we always render to a `canvas`, and then replace it with an `image` once rendering is done
Functionality wise, `ensureThumbnailVisible` is essentially just a shorthand for `scrollThumbnailIntoView`. (And note that `PDFViewer` doesn't implement a `ensurePageVisible` method.)
The only remaining usage of `PDFThumbnailViewer_ensureThumbnailVisible` is inside `PDFPresentationMode`, which introduces an otherwise unnecessary `PDFThumbnailViewer` dependency there.
We're already relying on the `presentationmodechanged` event, in various files, to track the state of Presentation Mode. Thus we can simply listen for that event in `PDFSidebar` too, and update the thumbnails if necessary.
*This is a follow-up to PRs 6299 and 6441.*
The patch also adds an option to `PDFThumbnailView`, that disables the canvas-to-image conversion entirely, which might be useful in the context of issue 7026.
This includes an optimization to zero the height and width of existing thumbnail canvases, when they are removed and recreated during rotation of the document. (Credit goes to nnethercote, who initially found this in PR 4920.)
