For named destinations that are contained in a `Dict`, as opposed to a `NameTree`, we currently iterate through the *entire* dictionary just to fetch *one* destination.
This code appears to simply have been copy-pasted from the `get destinations` method, but in its current form it's quite unnecessary/inefficient since can just get the required destination directly instead.
Doing this helped uncover an issue with the `getDestination` implementation.
Currently if a named destination doesn't exist, the method (in `obj.js`) may return `undefined` which leads to the promise being stuck in a pending state.
*Note:* returning `null` for this case is consistent with other methods, e.g. `getOutline` and `getAttachments`.
*The next step towards fixing issue 6158.*
We can just as well access `pdfViewer.currentScaleValue` directly in `PDFViewerApplication`, instead of having a helper function which just acts as a wrapper for it.
Currently if the zoom level is reset multiple times in a row, i.e. by pressing <kbd>Ctrl</kbd>+<kbd>0</kbd>, the pages can be re-rendered each time even though their size shouldn't change. Whether this happens can depend on the size of the viewer, but documents with pages in landscape mode seem to be very susceptible to this. (An example is: https://wiki.mozilla.org/images/5/55/MobileOpportunity.pdf.)
This can also effect documents with pages in portrait mode, when they are displayed in Presentation Mode.
The reason for this unnecessary re-rendering is that due to limited numerical precision, the new scale value may change in *only* the last decimal place.
This is similar to the already existing, separate, CSS file used for the `textLayer`, and it's necessary in order for the `pageviewer` components example to actually show annotations correctly.
Considering that most methods of `SimpleLinkService` are complete stubs, or practically "useless" considering what they return, we can actually simplify it even more.
*Note:* This depends on the previous patch, that did a small amount of refactoring of `PDFViewer_scrollPageIntoView`, since `PDFViewer.linkService.page` is no longer accessed.
----------
Currently the `pageviewer` components example doesn't work correctly (an error is printed in the console), since no `linkService` is present when the `AnnotationsLayerBuilder` is created.
*Note:* Given that this uses the `SimpleLinkService`, clicking on e.g. internal links won't actually do anything. However, given that internal links (and similar features) are pretty much useless when only *one* page is loaded the `pageviewer` example, I don't think that really matters.
Also, using the complete `PDFLinkService` would require a `PDFViewer` instance. That would significantly complicate the example, thus making it both less clear and less self contained.
Currently in `PDFViewer_scrollPageIntoView`, we're accessing a number of properties in an indirect and overly complicated way. In particular, using `this.linkService.page` is a *very* roundabout way to access `this.currentPageNumber`.
The reason for this appears to be entirely historical, since prior to PR 5361 the code was placed in `PDFPageView` (or `PageView` as it was called at the time).
This became obsolete in bdeca30fbf. All it does is call the Annotation contructor and add hasHtml. This patch lets the Link and Text annotations directly extend the Annotation class and add hasHtml themselves.
This patch also removes an unused global.
This patch is the the first step towards to addressing issue 6158, which will be done by refactoring the code for setting/getting the current scale in `viewer.js`.
As explained in
https://github.com/mozilla/pdf.js/issues/6174#issuecomment-118502802.
To verify that this patch works:
1. Build the Chrome extension (node make chromium)
2. Load the Chrome extension (at chrome://extensions)
3. Visit https://robwu.nl/pdfjs/issue6174/.
4. Verify that PDF.js is not used to load the PDF. Either Chrome's
default PDF Viewer is used, or the PDF is offered as a file download.
Before this patch, zooming in/out via the scroll wheel caused the page
to be zoomed relative to the upper-left corner of the page, i.e. the
upper-left corner of the page stays at a fixed position.
After this patch, the page is zoomed relative to the cursor position,
i.e. after zooming in/out, the part under the cursor 'has not moved'.
This only applies when the page does not fit in the viewport, because
pages smaller than the viewpoer are always centered.
In `viewer.js` we have code that ensures that the document is re-rendered on zooming and rotation. However, for `components` based viewers this might not work correctly, since there's currently no code which handles that.
Note that there is a good chance that this "just works" in many `components` viewers already, thanks to the `watchScroll` function.
The explanation is that zooming or rotation, most of the time, causes the viewer to change its (scrollable) size, thus triggering `PDFViewer_update` through `PDFViewer_scrollUpdate`.
However, in general there's no guarantee that this will actually work (since zooming and rotation doesn't necessarily change the size of the viewer for all documents), and requiring every viewer `components` implementer to provide methods for this doesn't seem like a great idea.
The patch adds a test to ensure that `getStats` returns the expected result after the page has been parsed, and cleans up the existing test a bit.
Also, since I'm touching the file anyway, I'm making a small adjustment of the `getDownloadInfo` test. (I have no idea why I didn't just write it like this initially.)
The reason that this code can be moved is that the `if (this.loading && page === 1)` check, in the `pagechange` event handler in viewer.js, is never satisfied since `this.loading` is not defined in that scope.
This *could* be considered a regression from PR 5295, since prior to that `this.loading` was using the `PDFViewerApplication` scope (or `PDFView` as it were).
However, I don't think that we need to fix that since we've been shipping this code in no less than *three* Firefox releases (uplifted in https://bugzilla.mozilla.org/show_bug.cgi?id=1084158), without breaking the world.
An explanation of why the `pagechange` code works, despite `this.loading === undefined`, is that `set currentPageNumber` (in `PDFViewer`) returns early whenever `this.pdfDocument` isn't set. This check is, for all intents and purposes, functionally equivalent to checking `PDFViewerApplication.loading`.
Hence we can move the page switching code into `PDFViewer`, and also remove `PDFViewerApplication.loading` since it's not used any more.
(The `this.loading` property was added in PR 686, which was before the current viewer even existed.)
*Note:* The changes in this patch should also be beneficial to the viewer `components`, since requiring every implementer to provide their own `pagechange` event handler just to get `PDFViewer.currentPageNumber` to actually work seems like an unnecessary complication.
