PR 13725 was only intended as a temporary work-around, and it seems that we can now revert that.
- Firefox 102 is the currently maintained ESR-branch, and the PDF.js project only supports the active one.
- Node.js now works, thanks to the `node-canvas` package, and I've confirmed locally that following the STR in issue 13724 generates a correct image.
- When we're editing some annotations, keeping the role="text-box" make them visible
as editable and VoiceOver (Mac) is able to read the contents when they're focused;
- Add an attribute "aria-activedescendant" in order to make the content discoverable
by NVDA on Windows.
Note how we're currently skipping all main-thread cleanup when document destruction has started, but for some reason we're still dispatching the "Cleanup" message.
This seems like a simple oversight, since destruction will already invoke the `BasePdfManager.cleanup` method (on the worker-thread) to fully clear-out all caches.
When a new PDF document is opened in the GENERIC viewer we (obviously) create a new `AnnotationEditorUIManager`-instance, since those are document-specific, and thus we need to ensure that we actually register the `editorTypes` for each one.
Looking at the code on the worker-thread, there doesn't appear to be any particular reason for placing *some* of the properties in a `source`-object when sending them with "GetDocRequest".
As is often the case the explanation for this structure is rather "for historical reasons", since originally we simply sent the `source`-object as-is. Doing that was obviously a bad idea, for a couple of reasons:
- It makes it less clear what is/isn't actually needed on the worker-thread.
- Sending unused properties will unnecessarily increase memory usage.
- The `source`-object may contain unclonable data, which would break the library.
Rather than sending all of these parameters individually and then grouping them together on the worker-thread, we can simply handle that in the API instead.
This patch first of all makes `isOffscreenCanvasSupported` configurable, defaulting to `true` in browsers and `false` in Node.js environments, with a new `getDocument` parameter. While you normally want to use this, in order to improve performance, it should still be possible for users to control it (similar to e.g. `isEvalSupported`).
The specific problem, as reported in issue 14952, is that the SVG back-end doesn't support the new ImageMask data-format that's introduced in PR 14754. In particular:
- When the SVG back-end is used in Node.js environments, this patch will "just work" without the user needing to make any code changes.
- If the SVG back-end is used in browsers, this patch will require that `isOffscreenCanvasSupported: false` is added to the `getDocument`-call.
*Please note:* The referenced issue is the only mention that I can find, in either GitHub or Bugzilla, of "GoToE" actions.
Hence why I've purposely settled for a very simple, and partial, "GoToE" implementation to avoid complicating things initially.[1] In particular, this patch only supports "GoToE" actions that references the /EmbeddedFiles-dict in the PDF document.
See https://web.archive.org/web/20220309040754if_/https://www.adobe.com/content/dam/acom/en/devnet/pdf/pdfs/PDF32000_2008.pdf#G11.2048909
---
[1] Usually I always prefer having *real-world* test-cases to work with, whenever I'm implementing new features.
This is yet another small piece of clean-up of the `FontLoader`-code, since we've not used this `id`-property for anything ever since PR 6571 (which landed almost seven years ago). Furthermore, by default we're also not even using that code-path now since the Font Loading API will always be used when available.
*Please note:* This is tagged `[api-minor]` since it's technically observable from the outside, however no user ought to be directly interacting with these CSS font rules.
Given that Firefox supports *synchronous* font loading, when the Font Loading API isn't being used, there's really no point including code which if called would just throw in the MOZCENTRAL build. (This is safe, since the `FontLoader.isSyncFontLoadingSupported`-getter always return `true` there.)
After the changes in PR 10539 (which landed over three years ago) the `FontLoader.bind` method can only be called with *a single* font at a time, hence the `_prepareFontLoadEvent` method obviously don't need to support multiple fonts any more.
By having just *one* class, and using pre-processor blocks directly in the relevant methods, we reduce the size of this code in the *built* `pdf.js` file.
Originally, when the `BaseFontLoader` abstraction was added in PR 9982, the idea was probably that additional build-targets would get their own implementations. Given that this hasn't happened in the four years since that landed, it doesn't seem meaningful to keep it around.
The existing `loadingContext` class-property can be simplified slightly, since we've not been using the `id`-property on the requests ever since PR 3477 (which landed nine years ago).
Furthermore, by default we're also not even using that code-path now since the Font Loading API will always be used when available.
This patch updates a bunch of older code, that makes conditional function calls, to use optional chaining rather than `if`-blocks.
These mostly mechanical changes reduce the size of the `gulp mozcentral` build by a little over 1 kB.
Note that this patch implements the `SetOCGState`-handling in `PDFLinkService`, rather than as a new method in `OptionalContentConfig`[1], since this action is nothing but a series of `setVisibility`-calls and that it seems quite uncommon in real-world PDF documents.
The new functionality also required some tweaks in the `PDFLayerViewer`, to ensure that the `layersView` in the sidebar is updated correctly when the optional-content visibility changes from "outside" of `PDFLayerViewer`.
---
[1] We can obviously move this code into `OptionalContentConfig` instead, if deemed necessary, but for an initial implementation I figured that doing it this way might be acceptable.
It slightly helps to reduce the code size and its complexity.
But the cool thing is that it allows to copy/paste some anntations from a pdf
to an other.
A number of Annotation-types are currently creating their own PopupAnnotations, since they need to use a custom `trigger`-element. However, because of where that check is currently implemented[1] we end up attaching empty/unused containers for those PopupAnnotations to the DOM[2]; see e.g. the `annotation-line.pdf` file in the test-suite for one example.
By instead moving the types-check into the `PopupAnnotationElement` constructor, we can completely skip those PopupAnnotations that are being explicitly handled elsewhere.
Note that I don't *believe* that this is a new issue, although I've not tried to bisect it, but this likely goes back quite some time (possibly even as far as PR 8228).
---
[1] In the `PopupAnnotationElement.render` method.
[2] Please note that the actual Popup-element *itself* isn't being attached/rendered here, just its container which by itself serves no purpose as far as I can tell.
There's three notable exceptions here:
- The `saveDocument` one is converted into a permanent `warn`, since it still works when the `annotationStorage` is empty although it's (obviously) less efficient than `getData`.
- The `fallbackWorkerSrc` functionality (for browsers), since just removing it would risk too much third-party breakage.
- The SVG back-end, since a final decision is yet to be made. (It might be completely removed, or left as-is in an essentially "frozen" state.)
Note that this patch prepends the document title with "* ", rather than only "*" as suggested in the bug, since there's nothing that says that a PDF document cannot specify a title[1] beginning with an asterisk. To reduce possible confusion, having a space between the "editing marker" and the actual document title thus cannot hurt as far as I'm concerned.
In order to notify the viewer when all `AnnotationEditor`s have been removed, we utilize the existing `onAnnotationEditor`-callback to allow the document title to be updated as necessary.
Finally, this patch makes the following (slightly unrelated) changes:
- Rename the `AnnotationStorage.removeKey` method to just `AnnotationStorage.remove` instead. This is consistent with e.g. the `has`-method and should suffice to explain what it does.
- Remove the `AnnotationStorage.hasAnnotationEditors` getter, since the viewer now tracks the necessary state internally. This avoids unnecessarily having to iterate through the `AnnotationStorage`-instance when saving/printing the document.
---
[1] Using either an /Info dictionary or a /Metadata stream.
This functionality has never been used anywhere in the PDF.js library/viewer itself, since it was added in 2013.
Furthermore this functionality is, and has always been, *completely untested* and also unmaintained.
Finally, there's (at least) one old issue about `appendImage` not returning the correct position; see issue 4182.
All-in-all, it seems that keeping very old, untested, unmaintained, and partially broken code around probably isn't what we want here.
(On the off-chance that any future a11y-work requires getting access to image-positions, it'd likely be much better to re-implement the necessary functionality from scratch and also make sure that it's properly tested from the beginning.)
This old method, which is only used with the `imageLayer` functionality, is essentially just a re-implementation of the existing `Util.applyTransform` method.
