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.
Apparently this is implemented in e.g. Adobe Reader, and the specification does support it, however it cannot be commonly used in real-world PDF documents since it took over ten years for this feature to be requested.
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.
The password dialog can be cancelled in three different ways:
- By clicking on its "Cancel"-button.
- By pressing the Escape-key.
- By force-opening another dialog, although this shouldn't happen in practice.
Here the "Cancel"-button case is slightly special since it'll trigger `PasswordPrompt.#cancel` *twice*, first directly via the click and secondly via the "close" event on the `dialog`-element.
While this shouldn't, as far as I know, cause any bugs it's nonetheless inconsistent with the other cases outlined above. To improve this we can simply attempt to *close* the password dialog instead, and then rely on the "close" event to run the `PasswordPrompt.#cancel` method.
Currently in `disableWorker=true` mode it's possible that opening of password-protected PDF documents outright fails, if an *incorrect* password is entered. Apparently the event ordering is subtly different in the non-Worker case, which causes the password-callback to be updated *before* the dialog has been fully closed.
To avoid that we'll utilize a `PromiseCapability` to keep track of the state of the password dialog, such that we can delay both re-opening and (importantly) updating of the password-callback until doing so is safe.
This patch *may* also fix issue 15330, but it's impossible for me to tell.
*This is a follow-up to PR 14869.*
In the old code we're accidentally "swallowing" part of the event-details, which explains why the annotationLayer didn't render.
One thing that made debugging a lot harder was the lack of error messages, from the viewer, and a few `PDFPageView`-methods were updated to improve this situation.
This was moved into the `src/display/`-folder in PR 15110, for the initial editor-a11y patch. However, with the changes in PR 15237 we're again only using `binarySearchFirstItem` in the `web/`-folder and it thus seem reasonable to move it back there.
The primary reason for moving it back is that `binarySearchFirstItem` is currently exposed in the public API, and we always want to avoid that unless it's either PDF-related functionality or code that simply must be shared between the `src/`- and `web/`-folders. In this case, `binarySearchFirstItem` is a general helper function that doesn't really satisfy either of those alternatives.
Currently when the `TextAccessibilityManager.enabled` method is called, we'll update `aria-owns` for any pre-existing elements. This obviously makes sense when e.g. zooming/rotating in the viewer, since the annotationLayer/annotationEditorLayer is kept in those cases.
However when the page is *fully* reset, e.g. as result of going out-of-view and thus being evicted from the cache, we still keep the `#textNodes`-Map around. This causes us to set the `aria-owns` attribute (in the textLayer) for an element that doesn't actually exist any more, which as far as I'm concerned seems incorrect. In this case the element will simply, as already implemented, be re-inserted when the annotationLayer/annotationEditorLayer renders again.
This patch doesn't structurally change the text layer: it just adds some aria-owns
attributes to some spans.
The aria-owns attribute expect to have an element id, hence it's why it adds back an
id on the element rendering an annotation, but this id is built in using crypto.randomUUID
to avoid any potential issues with the hash in the url.
The elements in the annotation layer are moved into the DOM in order to have them in the
same "order" as they visually are.
The overall goal is to help screen readers to present to the user the annotations as
they visually are and as they come in the text flow.
It is clearly not perfect, but it should improve readability for some people with visual
disabilities.
This exports the same constants as the viewer components, but in the default viewer. To avoid bloating the global-scope the constants are added to a new `PDFViewerApplicationConstants` object[1], which also allows us to skip this in builds where it's not actually needed (e.g. the Firefox *built-in* PDF Viewer).
*Please note:* I'm not completely sold on this idea, and thus wouldn't mind the patch being rejected, since we probably don't want to export every single viewer constant this way. (And it may seem a bit arbitrary, to users, why some constants are exported and others are not.)
---
[1] Somewhat similar to the existing `PDFViewerApplicationOptions` structure.
In addition to the existing `LinkTarget` constant, used by the `PDFLinkService`-constructor, this patch exports the following constants in the viewer components:
- `ScrollMode` and `SpreadMode`, since the `BaseViewer` has getters/setters which work with those constants.
- `RenderingStates`, since that one may be helpful when using `PDFPageView` directly.
By invoking the `reset` methods *last* in the `Toolbar`/`SecondaryToolbar`-constructors, we ensure that the "toolbarreset"/"secondarytoolbarreset"-events are actually handle when the viewer loads. Note that previously those events were dispatched *before* the relevant event-listeners had been attached.
With this small change we can avoid inconsistent initial toolbar-state, specifically in the case when the viewer is *reloaded* (since Firefox keeps the HTML-state on "soft" reloads).
An annotation doesn't have to be in the text flow, hence it's likely a bad idea
to insert its text in the text layer. But the text must be visible from a screen
reader point of view so it must somewhere in the DOM.
So with this patch, the text from a FreeText annotation is extracted and added in
a div in its HTML counterpart, and with the patch #15237 the text should be visible
and positioned relatively to the text flow.
Given that this image is intended specifically for the default viewer, we simply use the CSS preprocessor to remove the image reference in the `gulp components` build.
Considering that the issue only affects a CSS file, I don't believe that replacing the *just released* PDF.js version is actually necessary here.
It doesn't make sense to use a page-canvas that's *smaller* than the resulting thumbnail, since that causes the image to be upscaled which results in a blurry thumbnail. Note that this doesn't normally happen, unless a very small zoom-level is used in the viewer.
To improve performance of the sidebar we use the page-canvases to generate the thumbnails whenever possible, since that avoids unnecessary re-rendering when the sidebar is open. This works generally well, however there's an old problem in PDF documents that contain interactive forms (when those are enabled): Note how the thumbnails become partially (or fully) blank, since those Annotations are not included in the OperatorList.[1]
We obviously want to keep using the `PDFThumbnailView.setImage`-method for most documents, however we need a way to skip it only for those pages that contain interactive forms.
As it turns out it's unfortunately not all that simple to tell, after the fact, from looking only at the OperatorList that some Annotations were skipped. While it might have been possible to try and infer that in the viewer, it'd not have been pretty considering that at the time when rendering finishes the annotationLayer has not yet been built.
The overall simplest solution that I could come up with, was instead to include a *summary* of the interactive form-state when doing the final "flushing" of the OperatorList and expose that information in the API.
---
[1] Some examples from our test-suite: `annotation-tx2.pdf` where the thumbnail is completely blank, and `bug1737260.pdf` where the thumbnail is missing the "buttons" found on the page.
Currently all of these factories take a bunch of (randomly ordered) parameters, which first of all doesn't look that nice in the `PDFPageView`-class when some parameters are optional.
Furthermore, it also makes deprecation/removal of any existing parameter a *potentially* breaking change.
Finally, using an Object will provide a small amount of "documentation" at the call-site which isn't really the case with a bunch of "regular" parameters.
Note that all of the `viewer component` examples still work as-is with this patch, which is why I don't believe that we necessarily have to deprecate in the usual fashion.
Rather than always disable `PDFThumbnailView.setImage` as soon as user has changed the visibility of the Optional Content, we can utilize the new method added in the previous patch to improve thumbnail performance. Note in particular how, in the old code, even *resetting* of the Optional Content to its default state wouldn't enable `PDFThumbnailView.setImage` again.
While slightly unrelated, this patch also removes the `PDFThumbnailViewer._optionalContentConfigPromise`-property since it's completely unused.
Given that the SVG back-end is not defined anywhere except in GENERIC builds, we can remove a bit more unnecessary code in e.g. the Firefox PDF Viewer.
The elements in the annotationEditor layer are rearranged to make them
more accessible, but we must draw them in the order they have been created,
hence this patch adds a z-index to the editors.
- This way, the keyboard callbacks are called even if the page has not
the focus, hence the user doesn't have to guess that they have to click
on the page which is a bit painful especially in Ink mode.
- Add two keyboard shortcuts to commit a Freetext editor (ctrl+enter and
escape).
- In the annotationEditorLayer, reorder the editors in the DOM according
the position of the elements on the screen;
- add an aria-owns attribute on the "nearest" element in the text layer
which points to the added editor.
Given that the SVG back-end is not defined anywhere except in GENERIC builds, we can remove a little bit more unnecessary code in e.g. the Firefox PDF Viewer.
Rather than including all of this external code in the PDF.js repository, we should be using the npm package instead.
Unfortunately this is slightly more complicated than you'd hope, since the `fit-curve` package (which is older) isn't directly compatible with modern JavaScript modules.
In particular, the following cases needed to be considered:
- For the development viewer (i.e. `gulp server`) and the unit-tests, we thus need to build a fitCurve-bundle that can be directly `import`ed.
- For the actual PDF.js build-targets, we can slightly reduce the sizes by depending on the "raw" `fit-curve` source-code.
- For the Node.js unit-tests, the `fit-curve` package can be used as-is.
- this way the context menu in Firefox can take into account what we
have in the clipboard, if an editor is selected, ...
- when the user will click on a context menu item, an action will be
triggered, hence this patch adds what is required to handle it;
- some tests will be added in the Firefox' patch.
Rather than forcing the user to *manually* call `setDimensions`, which is also breaking any existing third-party code, it seems that we can simply let the `AnnotationLayer.{render, update}`-methods handle that internally.
As far as I can tell, based on testing manually in the viewer *and* running the browser-tests, everything still appears to work correctly with this patch.
- Simplify how we look-up the DOM-element, which should also be a tiny bit more efficent.
- Use private class-fields, rather than property-names prefixed with underscores.
- Inline the `#updateBar` helper-method directly in the `percent`-setter, since having a separate method doesn't seem necessary in this case.
- Set the `indeterminate`-class on the ProgressBar DOM-element, to simplify the code.
Finally, also (slightly) re-factors the `PDFViewerApplication.progress`-method to make it a bit smaller.
- for example in Dusk theme (Windows 11), black appears to be white, so
the user will draw something in white. But if they want to print or
save the used color must be black.
- fix a bug with the color input which only accepts hex string colors;
- adjust outline color of the selected/hovered editors in HCM.
Now that it's possible to change the font-size, this placeholder string feels a little bit long (especially for larger font-sizes).
Given that Editing is not enabled/released yet, I hope that it should be fine to update this without changing the l10n-id.
This replaces the boolean `annotationEditorEnabled` option/preference with a "proper" `annotationEditorMode` one. This way it's not only possible for the user to control if Editing is enabled/disabled, but also which *specific* Editing-mode should become enabled upon PDF document load.
Given that Editing is not enabled/released yet, I cannot imagine that changing the name and type of the option/preference should be an issue.
Similar to other recent patches, see e.g. PR 15057, we don't want to add these kind of `id`s to DOM-elements since they shouldn't become "linkable" through the URL hash.
*Please note:* This patch can be tested, in the viewer, with e.g. `bug1737260.pdf` from the test-suite.
- As in the annotation layer, use percent instead of pixels as unit;
- handle the rotation of the editor layer in allowing editing when rotation
angle is not zero;
- the different editors are rotated counterclockwise in order to be usable
when the main page is itself rotated;
- add support for saving/printing rotated editors.
Given that printing is triggered *synchronously* in browsers, it's thus possible for scripting (in PDF documents) to modify the Annotation-data while printing is currently ongoing.
To work-around that we add a new printing-specific `AnnotationStorage`, where the serializable data is *frozen* upon initialization, which the viewer can thus create/utilize during printing.
For encrypted PDF documents without the required permissions set, this patch adds support for disabling of Annotation-editing. However, please note that it also requires that the `pdfjs.enablePermissions` preference is set to `true` (since PDF document permissions could be seen as user hostile).[1]
As I started looking at the issue, it soon became clear that *only* trying to fix the issue without slightly re-factor the surrounding code would be somewhat difficult.
The following is an overview of the changes in this patch; sorry about the size/scope of this!
- Use a new `AnnotationEditorUIManager`-instance *for each* PDF document opened in the GENERIC viewer, to prevent user-added Annotations from "leaking" from one document into the next.
- Re-factor the `BaseViewer.#initializePermissions`-method, to simplify handling of temporarily disabled modes (e.g. for both Annotation-rendering and Annotation-editing).
- When editing is enabled, let the Editor-buttons be `disabled` until the document has loaded. This way we avoid the buttons becoming clickable temporarily, for PDF documents that use permissions.
- Slightly re-factor how the Editor-buttons are shown/hidden in the viewer, and reset the toolbar-state when a new PDF document is opened.
- Flip the order of the Editor-buttons and the pre-exising toolbarButtons in the "toolbarViewerRight"-div. (To help reduce the size, a little bit, for the PR that adds new Editor-toolbars.)
- Enable editing by default in the development viewer, i.e. `gulp server`, since having to (repeatedly) do that manually becomes annoying after a while.
- Finally, support disabling of editing when `pdfjs.enablePermissions` is set; fixes issue 15049.
---
[1] Either manually with `about:config`, or using e.g. a [Group Policy](https://github.com/mozilla/policy-templates).
- each annotation has its coordinates/dimensions expressed in percentage,
hence it's correctly positioned whatever the scale factor is;
- the font sizes are expressed in percentage too and the main font size
is scaled thanks a css var (--scale-factor);
- the rotation is now applied on the div annotationLayer;
- this patch improve the rendering of some strings where the glyph spacing
was not correct (it's a Firefox bug);
- it helps to simplify the code and it should slightly improve the update of
page (on zoom or rotation).
We want to avoid adding regular `id`s to Annotation-elements, since that means that they become "linkable" through the URL hash in a way that's not supported/intended. This could end up clashing with "named destinations", and that could easily lead to bugs; see issue 11499 and PR 11503 for some context.
Rather than using `id`s, we'll instead use a *custom* `data-element-id` attribute such that it's still possible to access the Annotation-elements directly.
Unfortunately these changes required updating most of the integration-tests, and to reduce the amount of repeated code a couple of helper functions were added.
This option is not used, nor has it ever been used, in the *built-in* Firefox PDF Viewer. Hence we can define it only for the environments where it makes sense instead.
This should really have been done as part of PR 12470, since it's now possible to directly set the `defaultUrl`-option without having to fallback to `var`-usage.
- Right now, we must select the tool, then click to select a page and
click to start drawing and it's a bit painful;
- so just create a new ink editor when we're hovering a page without one.
Given that the SVG back-end is not defined anywhere except in GENERIC builds, we can remove a little bit of unnecessary code in e.g. the Firefox PDF Viewer.
