This patch fixes an old inconsistency, when using `BasePreferences.{reset, set}`, where the internal Preference values would be kept even if writing them to storage failed.
Given that none of these fields were ever intended to be accessed directly from the *outside*, since that will lead to inconsistent/broken state, we can use modern ECMAScript features to ensure that they are indeed private.
*Please note:* I don't really know anything about a11y, hence it's possible that this patch either doesn't work correctly or at least isn't a complete solution.
In both the SecondaryToolbar and the Sidebar we have "button groups" that functionally acts essentially like radio-buttons. Based on skimming through [this MDN article](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/radio_role) it thus appears that we should tag them as such, using `role="radiogroup"` and `role="radio"`, and then utilize the `aria-checked` attribute to indicate to a11y software which button is currently active.
Unfortunately simply using `appearance: textfield;`, or even `-webkit-appearance: textfield;`, doesn't actually hide the "spinner" in number-input elements in e.g. Google Chrome.
Hence we need to use a work-around with the `-webkit-inner-spin-button` rule, however in our CSS code we also have `-webkit-outer-spin-button` currently. According to both [the MDN compatibility data](https://developer.mozilla.org/en-US/docs/Web/CSS/::-webkit-outer-spin-button#browser_compatibility) and also manual testing in Google Chrome Beta 99, the `-webkit-outer-spin-button` rule is no longer necessary and we can thus clean-up the CSS a tiny bit.
This was added in PR 4516 specifically for Safari on iOS devices, but according to MDN it should no longer be necessary now; see https://developer.mozilla.org/en-US/docs/Web/CSS/-webkit-overflow-scrolling#browser_compatibility
According to the MDN compatibility data, this CSS feature:
- Was never implemented anywhere *except* for Safari on iOS.
- Was never standardized and thus never existed in an *unprefixed* version.
- Has now been removed, starting with Safari version 13.
Given that [the FAQ](https://github.com/mozilla/pdf.js/wiki/Frequently-Asked-Questions#faq-support) already lists Safari as "Mostly" supported, and that the default viewer is written primarily for Mozilla Firefox, it ought to be fine to remove these CSS rules now.
At this point in time, after recent rounds of clean-up, the `webkit`-prefixed Fullscreen API is the only remaining *browser-specific* compatibility hack in the `web/`-folder JavaScript code.
The standard, and thus unprefixed, Fullscreen API has been supported for *over three years* in both Mozilla Firefox and Google Chrome. [According to MDN](https://developer.mozilla.org/en-US/docs/Web/API/Fullscreen_API#browser_compatibility), the unprefixed Fullscreen API has been available since:
- Mozilla Firefox 64, released on 2018-12-11; see https://wiki.mozilla.org/Release_Management/Calendar#Past_branch_dates
- Google Chrome 71, released on 2018-12-04; see https://en.wikipedia.org/wiki/Google_Chrome_version_history
Hence *only* Safari now requires using a prefixed Fullscreen API, and it's thus (significantly) lagging behind other browsers in this regard.
Considering that the default viewer is written *specifically* to be the UI for the Firefox PDF Viewer, and that we ask users to not just use it as-is[1], I think that we should only support the standard Fullscreen API now.
Furthermore, note also that the FAQ already lists Safari as "Mostly" supported; see https://github.com/mozilla/pdf.js/wiki/Frequently-Asked-Questions#faq-support
---
[1] Note e.g. http://mozilla.github.io/pdf.js/getting_started/#introduction
> The viewer is built on the display layer and is the UI for PDF viewer in Firefox and the other browser extensions within the project. It can be a good starting point for building your own viewer. *However, we do ask if you plan to embed the viewer in your own site, that it not just be an unmodified version. Please re-skin it or build upon it.*
Now that the there's ECMAScript support for properly private methods on `class`es, we can use that instead and thus remove all of the `@private` JSDocs comments.
In some cases, in the `PDFPageView` implementation, we're modifying the `sx`/`sy` properties when CSS-only zooming is being used.
Currently this requires that you remember to *manually* update the `scaled` property to prevent issues, which doesn't feel all that nice and also seems error-prone. By replacing the `scaled` property with a getter, this is now handled automatically instead.
The `CanvasRenderingContext2D.backingStorePixelRatio` property was never standardized, and only Safari set (its prefixed version of) it to anything other than `1`.
Note that e.g. MDN doesn't contain any information about this property, and one of the few sources of information (at this point) is the following post: https://stackoverflow.com/questions/24332639/why-context2d-backingstorepixelratio-deprecated
Hence we can simplify the `getOutputScale` helper function, by removing some dead code, and now it no longer requires any parameters when called.
Given that the `Navigator` interface has been available since "forever", please see https://developer.mozilla.org/en-US/docs/Web/API/Navigator#browser_compatibility, it's somewhat difficult to see why these checks are actually necessary since the viewer is only intended for usage in browsers.
Looking at the history of the code, this functionality was originally placed in the general `src/shared/compatibility.js` file which could thus run in e.g. worker-threads and Node.js environments (where the `Navigator` interface isn't available).
- it aims to fix#14562;
- 'X-\n' were not correctly positioned;
- when X is a diacritic (e.g. in "sä-\n", which is decomposed into "sa¨-\n") we must handle both things:
- diacritics on the one hand;
- "-\n" on the other hand.
This is essentially a *continuation* of PR 7926, where we added support for rejecting the current `PDFDocumentLoadingTask`-promise by throwing inside of the `onPassword`-callback.
Hence the naive way to address [bug 1754421](https://bugzilla.mozilla.org/show_bug.cgi?id=1754421) would be to simply throw in the `onPassword`-callback used in the default viewer. However it unfortunately turns out to not work, since the password input/validation is asynchronous, and we thus need another approach.
The simplest solution that I can come up with here, is thus to *extend* the `onPassword`-callback to also reject the current `PDFDocumentLoadingTask`-instance if an `Error` is explicitly passed as the input to the callback function. (This doesn't feel great, but I cannot see a better solution that isn't really complicated.)
Note that the *browser* findbar in Firefox uses "Title Case" for the labels, and it thus seem like a good idea to ensure that `PDFFindBar` in consistent with that.
Furthermore, the new label added in PR #13261 uses the "Title Case" format which means that currently the default viewer findbar looks inconsistent.
*Please note:* Based on the official Firefox localization docs, see https://firefox-source-docs.mozilla.org/l10n/overview.html#string-updates, changing only the casing should *not* require updating the key:
> 1) If the change is minor, like fixing a spelling error or case, the developer should update the en-US translation without changing the l10n-id.
When the viewer becomes narrow, the `PDFFindBar` will (forcibly) wrap its elements to prevent it from extending to the full window width.
Currently, after PR 13261, this now leads to the `findResultsCount` span taking up vertical space *unconditionally* when the findbar is wrapped. To avoid a blank space being shown in this case, before searching has begun, place the `findResultsCount` span in a "message" rather than an "options" container.
- get original index in using a dichotomic seach instead of a linear one;
- normalize the text in using NFD;
- convert the query string into a RegExp;
- replace whitespaces in the query with \s+;
- handle hyphens at eol use to break a word;
- add some \s* around punctuation signs
With these changes, we'll now *always* replace all whitespaces with standard spaces (0x20). This behaviour is already, since many years, the default in both the viewer and the browser-tests.
This `disableCreateObjectURL` option was originally introduced all the way back in PR 4103 (eight years ago), in order to work-around `URL.createObjectURL()`-bugs specific to Internet Explorer.
In PR 8081 (five years ago) the `disableCreateObjectURL` option was extended to cover Google Chrome on iOS-devices as well, since that configuration apparently also suffered from `URL.createObjectURL()`-bugs.[1]
At this point in time, I thus think that it makes sense to re-evaluate if we should still keep the `disableCreateObjectURL` option.
- For Internet Explorer, support was explicitly removed in PDF.js version `2.7.570` which was released one year ago and all IE-specific compatibility code (and polyfills) have since been removed.
- For Google Chrome on iOS-devices, while we still "support" such configurations, it's *not* the focus of any development and platform-specific bugs are thus often closed as WONTFIX.
Note here that at this point in time, the `disableCreateObjectURL` option is *only* being used in the viewer and any `URL.createObjectURL()`-bugs browser/platform bugs will thus not affect the main PDF.js library. Furthermore, given where the `disableCreateObjectURL` option is being used in the viewer the basic functionality should also remain unaffected by these changes.[2]
Furthermore, it's also possible that the `URL.createObjectURL()`-bugs have been fixed in *browser* itself since PR 8081 was submitted.[3]
Obviously you could argue that this isn't a lot of code, w.r.t. number of lines, and you'd be technically correct. However, it does add additional complexity in a few different viewer components which thus add overhead when reading and working with this code.
Finally, assuming the `URL.createObjectURL()`-bugs are still present in Google Chrome on iOS-devices, I think that we should ask ourselves if it's reasonable for the PDF.js project (and its contributors) to keep attempting to support a configuration if the *browser* developers still haven't fixed these kind of bugs!?
---
[1] According to https://groups.google.com/a/chromium.org/forum/#!topic/chromium-html5/RKQ0ZJIj7c4, which is linked in PR 8081, that bug was mentioned/reported as early as the 2014 (eight years ago).
[2] Viewer functionality such as e.g. downloading and printing may be affected.
[3] I don't have access to any iOS-devices to test with.
So that Firefox doesn't switch to pixel mode for compat with other
browsers.
This should fix https://github.com/mozilla/pdf.js/issues/14476, in terms
of restoring the previous behavior.
We probably want to change the pixel-based scrolling code to not scroll
so much (the deltaMode stuff normalizes to +/-1 tick for each wheel
event, perhaps the pixel-based value should do the same).
- it aims to fix issue #14307;
- this event has been added recently in Firefox and we can now use it;
- fix few bugs in aform.js or in annotation_layer.js;
- add some integration tests to test keystroke events (see `AFSpecial_Keystroke`);
- make dispatchEvent in the quickjs sandbox async.
*Please note:* This is a tentative patch, since I don't know if this is deemed important enough to fix.
The new event could be seen as a *supplement* to the existing "documentinit" and "documentloaded" events, but for the case when a PDF document fails to load.
To make the "documenterror" event generally useful, it'll include both the localized error message as well as the original reason for the error (when that exists).
This helper function has never been used in e.g. the worker-thread, hence its placement in `src/shared/util.js` led to a *small* amount of unnecessary duplication.
After the previous patches this helper function is now *only* used in the viewer, hence it no longer seems necessary to expose it through the official API.
*Please note:* It seems somewhat unlikely that third-party users were relying *directly* on this helper function, which is why it's not being exported as part of the viewer components. (If necessary, we can always change this later on.)
As part of the changes/improvement in PR 14092, we're no longer using the `addLinkAttributes` directly in e.g. the AnnotationLayer-code.
Given that the helper function is now *only* used in the viewer, hence it no longer seems necessary to expose it through the official API.
*Please note:* It seems somewhat unlikely that third-party users were relying *directly* on the helper function, which is why it's not being exported as part of the viewer components. (If necessary, we can always change this later on.)
This structure contains *almost* exclusively references to DOM elements (and a couple of simple strings), rather than complete classes/functions. Hence the `eventBus`-option sticks out a fair bit, and I'd guess that it's *mostly* unused in e.g. third-party implementations.
Given that we, in multiple places, mention that the default viewer shouldn't be used as-is I really don't think that we need to keep this special `eventBus`-option around. Furthermore, nowadays it's also a lot easier to (safely) access the existing `EventBus`-instance in the viewer; see https://github.com/mozilla/pdf.js/wiki/Third-party-viewer-usage#initialization-promise which shows how to listen for the default viewer being initialized (and its `eventBus` thus being available).
Besides converting `Catalog.getPageDict` to an `async` method, thus simplifying the code, this patch also allows us to pro-actively fix a existing issue.
Note how we're looking up References in such a way that `MissingDataException`s won't cause trouble, however it's *technically possible* that the entries (i.e. /Count, /Kids, and /Type) in a /Pages Dictionary could actually be indirect objects as well. In the existing code this could lead to *some*, or even all, pages failing to load/render as intended.
In practice that doesn't *appear* to happen in real-world PDF documents, but given all the weird things that PDF software do I'd prefer to fix this pro-actively (rather than waiting for a bug report).
With `Catalog.getPageDict` being `async` this is now really simple to address, however I didn't want to introduce a bunch more *unconditional* asynchronicity in this method if it could be avoided (since that could slow things down). Hence we'll *synchronously* lookup the *raw* data in a /Pages Dictionary, and only fallback to asynchronous data lookup when a Reference was encountered.
In addition to the above, this patch also makes the following notable changes:
- Let `Catalog.getPageDict` *consistently* reject with the actual error, regardless of what data we're fetching. Previously we'd "swallow" the actual errors except when looking up Dictionary entries, which is inconsistent and thus seem unfortunate. As can be seen from the updated unit-tests this change is API-observable, hence why the patch is tagged `[api-minor]`.
- Improve the consistency of the Dictionary /Type-checks in both the `Catalog.getPageDict` and `Catalog.getAllPageDicts` methods.
In `Catalog.getPageDict` there's a fallback code-path where we're *incorrectly* checking the /Page Dictionary for a /Contents-entry, which is wrong since a /Page Dictionary doesn't need to have a /Contents-entry in order to be valid.
For consistency the `Catalog.getAllPageDicts` method is also updated to handle errors in the /Type-lookup correctly.
- Reduce the `PagesCountLimit.PAUSE_EAGER_PAGE_INIT` viewer constant, to further improve loading/rendering performance of the *second* page during initialization of very long documents; PR 14359 follow-up.
*This addresses the following case missing from the previous patch:*
The viewer is loaded in an *active* window/tab, and enough time is allowed to pass in order to allow rendering to start. However, if the user then switches to another tab (or another program) *before* rendering has finished, the "load" event also needs to be unblocked.
Given that `requestAnimationFrame` is being used, see the `src/diplay/api.js` file, an inactive window/tab means that rendering will not run and we'll thus not fetch all pages. The latter is a requirement for the "load" event to be unblocked, in the MOZCENTRAL-version of, the default viewer.
This patch is a *partial* solution, since it only addresses the following situations:
- A *background* tab (containing the viewer) is reloaded, e.g. via the tab-bar context menu.
- The viewer is loaded in a active tab, but the user switches away from it (or switches to another program window) *before* rendering has started.
This provides a work-around for badly generated PDF documents that contain *negative* /FitH parameters (in the referenced issue the value `-32768` is used).
This patch, first of all, removes circular dependencies in the TypeScript definitions. Secondly, it also moves `RenderingStates` into `web/ui_utils.js` to break another type-dependency and directly use the `XfaLayerBuilder` during XFA-printing.
Finally, note that this patch *slightly* reduces the size of the default viewer (e.g. in the `MOZCENTRAL` build) by not having to bundle code which is completely unused.
This patch circumvents the issues seen when trying to update TypeScript to version `4.5`, by "simply" fixing the broken/missing JSDocs and `typedef`s such that `gulp typestest` now passes.
As always, given that I don't really know anything about TypeScript, I cannot tell if this is a "correct" and/or proper way of doing things; we'll need TypeScript users to help out with testing!
*Please note:* I'm sorry about the size of this patch, but given how intertwined all of this unfortunately is it just didn't seem easy to split this into smaller parts.
However, one good thing about this TypeScript update is that it helped uncover a number of pre-existing bugs in our JSDocs comments.
The size of the `web/ui_utils.js` file has increased over time, as more code has been added to (or moved into) that file. To reduce its size slightly, this patch moves the event-related functionality into a separate file.
In PR 14114 this was only added to the default viewer, which means that in the viewer components the user would need to *manually* implement /Lang handling. This was (obviously) a bad choice, since the viewer components already support e.g. structTrees by default; sorry about overlooking this!
To avoid having to make *two* `getMetadata` API-calls[1] very early during initialization, in the default viewer, the API will now cache its result. This will also come in handy elsewhere in the default viewer, e.g. by reducing parsing when opening the "document properties" dialog.
---
[1] This not only includes a round-trip to the worker-thread, but also having to re-parse the /Metadata-entry when it exists.
By making this API-call *unconditionally*, we introduce a (slight) delay in the initialization of *all* documents.
That seems quite unfortunate, since `pdfjs.enablePermissions` is off by default, and it thus seem better only do the API-call when actually needed; sorry about this!
For encrypted PDF documents without the required permissions set, this patch adds support for disabling of form editing. However, please note that it also requires that the `pdfjs.enablePermissions` preference is set to `true`[1] (since PDF document permissions could be seen as user hostile).
Based on https://www.adobe.com/content/dam/acom/en/devnet/pdf/pdfs/PDF32000_2008.pdf#G6.1942134, this condition hopefully makes sense.
---
[1] Either manually with `about:config`, or using e.g. a [Group Policy](https://github.com/mozilla/policy-templates).
This patch is essentially *another* continuation of PR 11263, which tried to improve loading/initialization performance of *very* large/long documents.
For most documents, unless they're *very* long, we'll eagerly initialize all of the pages in the viewer. For shorter documents having all pages loaded/initialized early provides overall better performance/UX in the viewer, however there's cases where it can instead *hurt* performance.
For documents with a couple of thousand pages[1], the parsing and pre-rendering of the *second* page of the document can be delayed (quite a bit). The reason for this is that we trigger `PDFDocumentProxy.getPage` for *all pages* early during the viewer initialization, which causes the worker-thread to be swamped with handling (potentially) thousands of `getPage`-calls and leaving very little time for other parsing (such as e.g. of operatorLists).
To address this situation, this patch thus proposes temporarily "pausing" the eager `PDFDocumentProxy.getPage`-calls once a threshold has been reached, to give the worker-thread a change to handle other requests.[2]
Obviously this may *slightly* delay the "pagesloaded" event in longer documents, but considering that it's already the result of asynchronous parsing that'll hopefully not be seen as a blocker for these changes.[3]
---
[1] A particularly problematic example is https://github.com/mozilla/pdf.js/files/876321/kjv.pdf (16 MB large), which is a document with 2236 pages and a /Pages-tree that's only *one* level deep.
[2] Please note that I initially considered simply chaining the `PDFDocumentProxy.getPage`-calls, however that'd slowed things down for all documents which didn't seem appropriate.
[3] This patch will *hopefully* also make it possible to re-visit PR 11312, since it seems that changing `Catalog.getPageDict` to an `async` method wasn't the problem in itself. Rather it appears that it leads to slightly different timings, thus exacerbating the already existing issues with the worker-thread being overloaded by `getPage`-calls.
Having recently worked with that method, there's a couple of (very old) issues that I'd also like to address and having `Catalog.getPageDict` be `async` would simplify things a great deal.
These changes improves the consistency ever so slightly in the `PDFOutlineViewer._dispatchEvent` method, by making sure that we can tell the following two cases apart:
- The "pagesloaded" event has *not yet* been fired.
- The "pagesloaded" event has been fired, but no pages were available.
*This patch can be tested e.g. with the `poppler-85140-0.pdf` document from the test-suite.*
For some sufficiently corrupt documents the `getDocument` call will succeed, but fetching even the very first page fails. Currently we only print error messages (in the console) from the `{BaseViewer, PDFThumbnailViewer}.setDocument` methods, but don't actually provide these errors to allow the viewer to handle them properly.
In practice this means that the GENERIC viewer won't display the `errorWrapper`, and in the MOZCENTRAL viewer the *browser* loading indicator is never hidden (since we never unblock the "load" event).
This patch is essentially a continuation of PR 11263, which tried to improve loading/initialization performance of *very* large/long documents.
Note that browsers, in general, don't handle a huge amount of DOM-elements very well, with really poor (e.g. sluggish scrolling) performance once the number gets "large". Furthermore, at least in Firefox, it seems that DOM-elements towards the bottom of a HTML-page can effectively be ignored; for the PDF.js viewer that means that pages at the end of the document can become impossible to access.
Hence, in order to improve things for these *very* large/long documents, this patch will now enforce usage of the (recently added) PAGE-scrolling mode for these documents. As implemented, this will only happen once the number of pages *exceed* 15000 (which is hopefully rare in practice).
While this might feel a bit jarring to users being *forced* to use PAGE-scrolling, it seems all things considered like a better idea to ensure that the entire document actually remains accessible and with (hopefully) more acceptable performance.
Fixes [bug 1588435](https://bugzilla.mozilla.org/show_bug.cgi?id=1588435), to the extent that doing so is possible since the document contains 25560 pages (and is 197 MB large).
This was added on the assumption that the viewer would (eventually) start using the `PDFSinglePageViewer` for e.g. PAGE-scrolling mode and PresentationMode. However, having both a `PDFViewer` and a `PDFSinglePageViewer` side-by-side in the viewer would've been tricky to implement well, which is why PR 14112 implemented PAGE-scrolling for the general `BaseViewer` instead.
Given that the default viewer is no longer (potentially) going to use `PDFSinglePageViewer`, there's code in the `SecondaryToolbar` (and related CSS rules) which is now unnecessary.
*This patch can be tested e.g. with the `sizes.pdf` document in the test-suite.*
While this patch isn't necessarily the best solution, e.g. it might be possible to solve this with *only* CSS, it's what I was able to come up with to address an old issue.
The solution here re-uses the `spread`-class in PresentationMode, since that one already takes care of centering pages *vertically*, together with a dummy-page that takes up the entire height of the window.
Finally, some PresentationMode-related CSS-rules are also simplified slightly, since the changes in PR 14112 (using Page-scrolling) allows some clean-up here.
The issue that this patch fixes has existed ever since the viewer was first re-factored into components, however it only really affects the `disableAutoFetch = true` mode.
By default we're fetching all pages in `BaseViewer.setDocument`, and as part of the parsing/initialization we're also populating the `PDFLinkService`-pagesRefCache. The purpose of that cache is to make navigating to any internal destinations faster, by not having to (asynchronously) lookup the pageNumber via the API when handling the destination.
In comparison, when the `disableAutoFetch = true` mode is being used we're instead *lazily* initializing the pages in the `BaseViewer.#ensurePdfPageLoaded`-method. For some reason, that I can only assume is a simple oversight, we're not attempting to update the `PDFLinkService`-pagesRefCache in that case.
In the `BaseViewer` this cache is mostly relevant in the `disableAutoFetch = true` mode, since the pages are being initialized *lazily* in that case.
In the `PDFThumbnailViewer` this cache is mostly used for thumbnails that are actually being rendered, as opposed to those created directly from the "regular" pages.
Please note that I'm not suggesting that we remove these caches because they're only used in some situations, but rather because they're for all intents and purposes actually *redundant*. In the API itself, we're already caching both the page-promises and the actual pages themselves on the `WorkerTransport`-instance.
Hence these viewer-caches aren't really necessary in practice, and adds what to me mostly seems like an unnecessary level of indirection.[1]
Given that the viewer now relies on caching in the API itself, this patch also adds a new unit-test to ensure that page-caching works (and keep working) as expected.
---
[1] In the `WorkerTransport.getPage`-method the parameter is being validated on every call, but that's hardly enough code to warrant keeping the "duplicate" caches in the viewer in my opinion.
*Please note:* These changes will primarily benefit longer documents, somewhat at the expense of e.g. one-page documents.
The existing `PDFDocumentProxy.getStats` function, which in the default viewer is called for each rendered page, requires a round-trip to the worker-thread in order to obtain the current document stats. In the default viewer, we currently make one such API-call for *every rendered* page.
This patch proposes replacing that method with a *synchronous* `PDFDocumentProxy.stats` getter instead, combined with re-factoring the worker-thread code by adding a `DocStats`-class to track Stream/Font-types and *only send* them to the main-thread *the first time* that a type is encountered.
Note that in practice most PDF documents only use a fairly limited number of Stream/Font-types, which means that in longer documents most of the `PDFDocumentProxy.getStats`-calls will return the same data.[1]
This re-factoring will obviously benefit longer document the most[2], and could actually be seen as a regression for one-page documents, since in practice there'll usually be a couple of "DocStats" messages sent during the parsing of the first page. However, if the user zooms/rotates the document (which causes re-rendering), note that even a one-page document would start to benefit from these changes.
Another benefit of having the data available/cached in the API is that unless the document stats change during parsing, repeated `PDFDocumentProxy.stats`-calls will return *the same identical* object.
This is something that we can easily take advantage of in the default viewer, by now *only* reporting "documentStats" telemetry[3] when the data actually have changed rather than once per rendered page (again beneficial in longer documents).
---
[1] Furthermore, the maximium number of `StreamType`/`FontType` are `10` respectively `12`, which means that regardless of the complexity and page count in a PDF document there'll never be more than twenty-two "DocStats" messages sent; see 41ac3f0c07/src/shared/util.js (L206-L232)
[2] One example is the `pdf.pdf` document in the test-suite, where rendering all of its 1310 pages only result in a total of seven "DocStats" messages being sent from the worker-thread.
[3] Reporting telemetry, in Firefox, includes using `JSON.stringify` on the data and then sending an event to the `PdfStreamConverter.jsm`-code.
In that code the event is handled and `JSON.parse` is used to retrieve the data, and in the "documentStats"-case we'll then iterate through the data to avoid double-reporting telemetry; see https://searchfox.org/mozilla-central/rev/8f4c180b87e52f3345ef8a3432d6e54bd1eb18dc/toolkit/components/pdfjs/content/PdfStreamConverter.jsm#515-549
It shouldn't be necessary to iterate through *all* pages when using a non-default `spreadMode`, since we already know which page(s) should become visible.
This code is a left-over from the initial (local) implementation that resulted in PR 14112, however I forgot to clean-up some things such as e.g. this loop.
Also fixes an outdated comment, see PR 14204 which removed the mentioned data-structure.
This patch preserves the old behaviour of appending a `loadingIcon`-div to all pages that are not yet loaded/rendered. However, the actual `loadingIcon`-spinner (i.e. the `loading-icon.gif` image) will only be displayed on *visible* pages to improve performance.
To avoid having to iterate through all pages in the document, which doesn't seem like a good idea for a PDF document with thousands of pages, we use a combination of the currently visible *and* cached pages to toggle the `loadingIcon`-spinner.
This code is the last piece[1] of the viewer that's not using standard `class`es, and by converting this code we get rid of some now unneeded boilerplate code (slightly reducing the size of the *built* `web/viewer.js` file).
Note that while this code was originally imported from a separate repository, it was last sync-ed with upstream *five years* ago which is why this re-factoring should be OK as far as I'm concerned (and we've done some other clean-up since then as well).
---
[1] Technically the `web/debugger.js` file is left as well, however that code is first of all not bundled in the *built* `web/viewer.js` file and secondly it's not even loaded by default either.
- First step to fix https://bugzilla.mozilla.org/show_bug.cgi?id=1737260;
- several interactive pdfs use the possibility to hide/show buttons to show different icons;
- render pushbuttons on their own canvas and then insert it the annotation_layer;
- update test/driver.js in order to convert canvases for pushbuttons into images.
Reporting telemetry, in Firefox, includes using `JSON.stringify` on the data and then sending an event to the `PdfStreamConverter.jsm`-code.
In that code the event is handled and `JSON.parse` is used to retrieve the data, and in the "pageInfo"-case we'll then proceed to ignore everything except *the first* such event; see https://searchfox.org/mozilla-central/rev/24fac1ad31fb9c6e9c4c767c6a7ff45d226078f3/toolkit/components/pdfjs/content/PdfStreamConverter.jsm#509-514
All-in-all, sending the "pageInfo" telemetry for each rendered page is thus unnecessary and this patch makes the viewer send it only *once* instead.
*This is a tentative patch, since we unfortunately cannot easily test it (as far as I can tell).*
In Firefox this (obviously) works as-is, but in Google Chrome the `markedContent` spans are inserted within the regular text-content (in the DOM) and with non-zero heights.
*This is a tentative patch, since I don't have the necessary hardware to test it.*
See https://developer.mozilla.org/en-US/docs/Web/CSS/text-size-adjust, which is currently ignored in Firefox.
It seems overall safer, and more future-proof, to simply add this to the *entire* `textLayer` rather than its individual elements.
With the previous patch, this helper function is no longer used and keeping it around will simply increase the size of the builds.
This removal is purposely done *separately*, to make it easy to revert the patch in the future if this helper function would become useful again.
This relies on the fact that `Set`s preserve the insertion order[1], which means that we can utilize an iterator to access the *first* stored view.
Note that in the `resize`-method, we can now move the visible pages to the back of the buffer using a single loop (hence we don't need to use the `moveToEndOfArray` helper function any more).
---
[1] This applies to `Map`s as well, although that's not entirely relevant here.
The `PDFPageViewBuffer`-code is very important for the correct function of the viewer, but it's currently not tested at all.
While the `PDFPageViewBuffer` is obviously intended to be used with `PDFPageView`-instances, it only accesses a couple of `PDFPageView` properties/methods and consequently it's fairly easy to unit-test this code with dummy-data.
These unit-tests should help improve our confidence in this code, and will also come in handy with other changes that I'm working on (regarding modernizing and re-factoring the `PDFPageViewBuffer`-code).
The way that we're currently handling the last-`id` is very old, and there's no longer any good reason to special-case things when only one thumbnail is visible.
Furthermore, we can also modernize the loop slightly by using `for...of` instead of `Array.prototype.some()` when checking for fully visible thumbnails.
Note how in `PDFPageViewBuffer.resize` we're manually iterating through the visible pages in order to build a Set of the visible page `id`s. By instead moving the building of this Set into the `getVisibleElements` helper function, as part of the existing parsing, this code becomes *ever so slightly* more efficient.
Furthermore, more direct access to the visible page `id`s also come in handy in other parts of the viewer as well.
In the `BaseViewer.isPageVisible` method we no longer need to loop through the visible pages, but can instead directly check if the pageNumber is visible.
In the `PDFRenderingQueue.getHighestPriority` method, when checking for "holes" in the page layout, we can also avoid some unnecessary look-ups this way.
*Sometimes I'll hopefully learn to optimize my code directly when writing it, rather than having to do multiple clean-up passes; sorry about the churn here!*
For most page layouts there won't be any "holes" in the visible pages (or thumbnails), and in those cases it'd obviously be preferable not having to repeat any checks of already rendered pages.
Rather than only checking the "distance" between the first/last pages, we can instead compare the theoretical number of pages (between first/last) with the actually visible number of pages instead. This way, we're able to better detect the "holes"-case and can skip unnecessary parsing in the common case.
I missed this one spot in PR 12870, when converting the other cases in the "keydown" event handler. However, given that it only matters in PresentationMode and/or when "page-fit" zooming is enabled, this oversight shouldn't have had any user-observable impact (but we should fix it nonetheless).
This is a follow-up to PR 10675, since there I completely overlooked that we also need to handle the case where a PDF document has *failed* to load when the "supportsRangedLoading" message is sent to the viewer.
There were some links not working in some XFA files,I realized that the anchor tag that contains the link has an inline display and couldn't receive any height, solved this by adding a "position: absolute". Tested with two different files in Firefox Nightly and Chrome and now all links are working perfectly fine. Added reftest to avoid future regressions
The only reason for using a `DocumentFragment` in the first place, originally added in PR 8724, was to prevent errors in the `PDFPageView`-constructor. However, we should be able to simply make its `container`-option *optional* instead, since it's not being used for anything else in the class.
Note that pre-rendering still works correctly in my testing, and given that the `BaseViewer` keeps references to all `PDFPageView`-instances (via its `_pages` Array) it also shouldn't be possible to "lose" any pages/canvases this way.
Unfortunately there exist PDF documents where all pageLabels are empty strings, see e.g. http://www.cs.cornell.edu/~ragarwal/pubs/blk-switch.pdf (taken from an old issue), which result in the pageNumber-input being completely blank. That doesn't seem very helpful, and this patch simply extends the approach used to ignore pageLabels that are identical to standard page numbering.
In the GENERIC viewer, e.g. when dragging-and-dropping a new PDF document which automatically opens the outline, there can now be breaking errors in the `{BaseViewer, PDFThumbnailViewer}.#getScrollAhead` methods since there's no visible pages/thumbs during loading; sorry about the breakage!
This was a stupid oversight on my part, since the first/last visible pages have obviously already been rendered at the point when we're checking for any potential "holes" in the page layout.
While this will obviously not have any measurable effect on performance, we should nonetheless avoid doing completely unnecessary checks here.
This is a very old "issue", which has existed since essentially forever, and it affects all of the available scrollModes. However, in the recently added Page-mode it's particularily noticeable since we use a *simulated* scroll direction there.
When deciding what page(s) to pre-render, we only consider the current scroll direction. This works well in most cases, but can break down at the start/end of the document by trying to pre-render a page *outside* of the existing ones. To improve this, we'll thus *force* the scroll direction at the start/end of the document.
*Steps to reproduce:*
0. Open the viewer, e.g. https://mozilla.github.io/pdf.js/web/viewer.html
1. Enable vertical scrolling.
2. Press the <kbd>End</kbd> key.
3. Open the devtools and, using the DOM Inspector, notice how page 13 is *not* being pre-rendered.
*Please note:* This is a tentative patch, since I don't have the necessary a11y-software to actually test it.
To avoid having to add a new API-method just for a single string, I figured that adding the new property to the existing `documentInfo`-data (accessed via `PDFDocumentProxy.getMetadata` in the API) will hopefully be deemed acceptable.
Looking at the code, I do have to agree with the point made in issue 12731 about it being unexpected/unhelpful that the `PDFFindController.executeCommand`-method isn't directly usable with the "find"-event.
The reason for it being this way is, as so often, for historical reasons: The `executeCommand`-method was added (just) prior to the introduction of the `EventBus` in the viewer.
Obviously we cannot simply change the existing `PDFFindController.executeCommand`-method, since that'd be a breaking change in code which has existed for over five years.
Initially I figured that we could simply add a new method in `PDFFindController` that'd accept the state from the "find"-event, however after thinking about this and looking through the use-cases in the default viewer I settled on a slightly different approach: Let the `PDFFindController` just listen for the "find"-event (on the `EventBus`-instance) directly instead, which also removes one level of (unneeded) indirection during searching in the default viewer.
For GENERIC builds of the PDF.js library, the old `PDFFindController.executeCommand`-method is still available with a deprecation warning.
Many years ago now there were some `Promise` implementations that had issues resolving with an *implicitly* `undefined` value. That should no longer be the case, and we've not been using the `Promise.resolve(undefined)` format for a long time, hence this patch fixes the few remaining cases.
Having recently worked with this code, in PR 14096 (and indirectly in PR 14112), I happened to notice a pre-existing issue with spreadModes at higher zoom levels.
The `PDFRenderingQueue` code was written back when the viewer only supported "normal" vertical scrolling, and some edge-cases related to spreadModes are thus not perfectly supported. Depending on the zoom level, it's possible that there are "holes" in the currently visible page layout, and those pages will not be pre-rendered as you'd expect.
*Steps to reproduce:*
0. Open the viewer, e.g. https://mozilla.github.io/pdf.js/web/viewer.html
1. Enable vertical scrolling.
2. Enable the ODD spreadMode.
3. Scroll down, such that both pages 1 and 3 are visible.
4. Zoom-in until *only* page 1 and 3 are visible.
5. Open the devtools and, using the DOM Inspector, notice how page 2 is *not* being pre-rendered despite all surrounding pages being rendered.
With the previous commit, both of the `PDFViewer` and `PDFSinglePageViewer` clases are now small/simple enough that it no longer seems necessary to keep them in separate files.
This implements a new Page scrolling mode, essentially bringing (and extending) the functionality from `PDFSinglePageViewer` into the regular `PDFViewer`-class. Compared to `PDFSinglePageViewer`, which as its name suggests will only display one page at a time, in the `PDFViewer`-implementation this new Page scrolling mode also support spreadModes properly (somewhat similar to e.g. Adobe Reader).
Given the size and scope of these changes, I've tried to focus on implementing the basic functionality. Hence there's room for further clean-up and/or improvements, including e.g. simplifying the CSS/JS related to PresentationMode and implementing easier page-switching with the mouse-wheel/arrow-keys.
Note that PR 14049 removed this, since https://github.com/mozilla/pdf.js/pull/14049#discussion_r716245518 claimed that it's not necessary anymore. Unfortunately that, in my testing on Windows, actually re-introduced exactly the issue described in the comment; more specifically once the *last* character has been entered in the comb-field it's now again incorrectly scrolled (with the first character being invisible) until focus is lost.
This can be tested with e.g. `f1040.pdf`, see page 2, from the test-suite.
This patch helps reduce some duplication, given that we now have a few essentially identical `addLinkAttributes` call-sites in the code-base.
To prevent runtime errors in the Annotation/XFA-layer code, we'll warn if a custom/incomplete `PDFLinkService` is being used (limited to GENERIC builds).
Please note that we (obviously) don't want to unconditionally pre-render more than one page all the time, since that could very easily lead to overall worse performance in some documents.[1]
However, when spreadModes are enabled it does make sense to attempt to pre-render both of the pages of the next/previous spread.
---
[1] Since it may cause pre-rendering to unnecessarily compete for parsing resources, on the worker-thread, with "regular" rendering.
Note how both the annotationLayer and the document outline will apply various URL-related options when creating the link-elements.
For consistency the `xfaLayer`-rendering should obviously use the same options, to ensure that the existing options are indeed applied to all URLs regardless of where they originate.
- it aims to fix https://bugzilla.mozilla.org/show_bug.cgi?id=1716758;
- some buttons have a JS action with the pattern `app.launchURL(...)` (or similar) so extract when it's possible the url and generate a <a> element with the href equals to the found url;
- pdf.js already had some code to handle that so this patch slightly refactor that.
