Looking at the save-telemetry values they're all boolean *except* for "alt_text_edit" in one instance, since `this.#previousAltText` may be an empty string (looking at the `editAltText` method) and this value may thus become an empty string as well.
When closing a document in the viewer, e.g. by running `PDFViewerApplication.close()` in the console, the `AltTextManager.#finish` method currently throws *unless* the `altText` dialog is actually open.
Similar to e.g. the PasswordPrompt, we should thus only attempt to close the `altText` dialog when it's open.
It's a part of the UX specifications. There's a drawing issue in Firefox
(see bug https://bugzilla.mozilla.org/1853288) but setting the
background-clip property to content-box seems to be a good workaround.
The goal is to always have something which is focusable to let the user select
it with the keyboard.
It fixes the mentioned bug because, the annotation layer will now have a container
to attach the canvas for annotations having their own canvas.
`.grab-to-pan-grab:active` is `#viewerContainer` when the mouse is
pressed down. It is supposed to have a `cursor: grabbing` appearance
immediately on mousedown,
`.grab-to-pan-grabbing` is the overlay that is supposed to cover
everything, and also has the `cursor: grabbing` appearance. The "cover
everything" result is achieved through `position:fixed`, `inset:0`, etc.
The block with these CSS properties for "cover everything" is currently
shared by `.grab-to-pan-grab:active` and `.grab-to-pan-grabbing`, but
only "cursor" need to be shared. The original JS and CSS code at
https://github.com/Rob--W/grab-to-pan.js shows that these were supposed
to be associated with the overlay only.
The PR that added this to PDF.js also shows that the "cover everything"
CSS properties were supposed to be limited to the overlay only:
https://github.com/mozilla/pdf.js/pull/4209#discussion-diff-9285917
But the final version of the PR mistakenly merged them together.
This patch rectifies that mistake.
Using `removeNullCharacters` on the URL should be completely redundant, given the kind of data that we're passing to the `addLinkAttributes` helper function. Note that whenever we're handling a URL, originating in the worker-thread, in the viewer that helper function is always being used.
Furthermore, on the worker-thread all URLs are parsed with the `createValidAbsoluteUrl` helper function, which uses `new URL()` to ensure that a valid URL is obtained. Note that the `URL` constructor will either throw, or in some cases just ignore them, when encountering `\u0000`-characters during parsing.
Hence it should be *impossible* for a valid URL to contain `\u0000`-characters and we can thus simplify the viewer-code a tiny bit. The use of `removeNullCharacters` is most likely a left-over from back when `new URL()` wasn't generally available in browsers.
Testing the `tagged_stamp.pdf` document locally in the viewer, I noticed that e.g. the /Alt entry for the StampAnnotation contains "Secondary text for stamp\u0000".
Elsewhere in the viewer we're skipping null-chars and it's easy enough to do that in the `StructTreeLayerBuilder` class as well. (Note that we generally let the API itself return the data as-is.)
This fixes invalid type references (either due to invalid paths for the
import or missing imports) in the JS doc, as well as some missing or
invalid parameter names for @param annotations.
The main stamp button will be used to just enter in a add/edit image mode:
- the user can add a new image in using the new button.
- the user can edit an image in resizing, moving it.
In image mode, when the user clicks outside on the page but not on an editor,
then all the selected editors will be unselected.
After the `src/core/`-changes in PR 16779 the `PDFDocumentProxy.getJSActions` method should no longer be able to return *empty* entries, which means that we can simplify the "JavaScript support is not enabled"-warning in the viewer.
Furthermore, improve the auto-printing hack used when scripting is disabled.
There are 2 rotation we've to deal with: the viewer one and the editor one.
The previous implementation was a bit complex and having to deal with these
rotation would have potentially increase it.
So this patch aims to simplify the implementation and deal with all the possible
cases.
The main idea is to transform the mouse deltas according to the rotations and then
apply the resizing in the page coordinates system.
When resizing an editor we're currently using unidirectional cursors, please refer to https://developer.mozilla.org/en-US/docs/Web/CSS/cursor
Given that editors can (generally) be resized to become either smaller or larger, it seems overall more appropriate to use bidirectional cursors to make this clearer to the user.
Note that as mentioned in the MDN article some environments, which seems to apply to e.g. Windows 11, doesn't differentiate between the two cursor formats and simply use bidirectional ones unconditionally.
One additional benefit of these changes is that the relevant CSS rules become slightly more compact.
This method is very old, however with the exception of the auto-print hack (when scripting is disabled) in the viewer it's never actually been used.
Most likely the idea with `PDFDocumentProxy.getJavaScript` was that it'd be useful if scripting support was added, however it turned out that it was a bit too simplistic and instead a number of new methods were added for the scripting use-cases.
Without this patch the password dialog is pretty difficult to use in the GeckoView-viewer, because of a number of missing CSS variables.
*Please note:* This patch makes no effort at actually styling the dialog to better suite the overall look of the GeckoView-viewer, but focuses solely on making it actually usable (since password protected PDF documents are somewhat rare).
If the current PDF document is closed while the password dialog is open, e.g. manually by calling `PDFViewerApplication.close()` from the console, the password dialog wouldn't be closed as intended.
*Please note:* This could only affect the GENERIC viewer, although it's very unlikely to ever happen, since that's the only one that supports opening more than one PDF document.
*Please note:* This situation should never happen in practice, but it nonetheless cannot hurt to fix this.
If the `PasswordPrompt.open` method would ever be called synchronously back-to-back *and* if opening of the dialog fails the first time, then the second invocation would remain pending indefinitely since we just clear out the capability.
Given that the `useOnlyCssZoom` option is essentially just a special-case of the `maxCanvasPixels` functionality, we can combine the two options in order to simplify the overall implementation.
Note that the `useOnlyCssZoom` functionality was only ever used, by default, in the PDF Viewer for the B2G/FirefoxOS project (which was abandoned years ago).
This is quite old code, however the error-handling no longer seems necessary for a couple of reasons:
- The `PDFViewerApplication.open` method is asynchronous, which means that it cannot throw a "raw" `Error` and the try-catch is not needed in that case.
- None of the other affected methods should throw, and if they do that'd rather indicate an *implementation* error in the code.
- Finally, and most importantly, with the `PDFViewerApplication.run` method now being asynchronous an (unlikely) `Error` thrown within it will lead to a rejected `Promise` and not affect execution of other code.
We can use modern JavaScript features, in this case optional chaining, to (ever so slightly) simplify how `ViewHistory` errors are handled.
Also, use arrow functions when handling a few other (very rare) errors during loading since that's a tiny bit shorter.
Given that the `debugger` is loaded as a module we can use "top level await" in development mode to access the necessary API-functionality, which removes the need to manually pass in the required properties.
- it'll improve the way to resize images: diagonally (in keeping ratio between dimensions)
or horizontally/vertically.
- the resizer was almost invisible in HCM.
- make a resize undoable/redoable.
In order to reproduce the original issue:
- switch to freetext mode
- add a text somewhere
- double click outside and add some text
- repeat the previous step several times
no text is selected during the edition.
*Please note:* This only removes the preference itself, however both the viewer-option and the actual implementation is still available.
The `useOnlyCssZoom` functionality was only ever used, by default, in the PDF Viewer for the B2G/FirefoxOS project (which was abandoned years ago). Given that CSS-only zooming can easily make the document look blurry even at low zoom levels, this functionality was only intended for low-powered mobile devices.
Hence it seems reasonable to remove the `useOnlyCssZoom` preference now, since neither the default viewer nor the GeckoView-specific viewer uses this functionality.
By leveraging import maps we can get rid of *most* of the remaining `require`-calls in the `src/display/`-folder, since we should strive to use modern `import`-statements wherever possible.
The only remaining cases are Node.js-specific dependencies, since those seem very difficult to convert unless we start producing a bundle *specifically* for Node.js environments.
Localization of this button broke in PR 16340, which I assume was completely accidental, since the download-button now tries to access a l10n-id that was removed some time ago (see PR 15617).
Note how loading even the development viewer, i.e. http://localhost:8888/web/viewer-geckoview.html#locale=en-US, currently logs l10n-warnings on the `master` branch.
Having a `require` in this file has never made sense in e.g. the Firefox PDF Viewer and shouldn't really be necessary.
Possibly the idea was to facilitate some kind of third-party bundling, however the *built* `pdf.js` file has always exposed the API-contents globally.
Currently this class contains a few "special" code-paths for the COMPONENTS build-target, which normally wouldn't be a problem. However, in this particular case that means accessing code that we don't want to include unconditionally in all builds.
This is currently implemented using build-time `require`-calls which we nowadays want to avoid, and we should strive to remove all such cases from the code-base. (Generally speaking `import` is the future, and build-tools may not always play well with a mix of both formats.)
We can easily improve things here by using sub-classing for the COMPONENTS build-target, and then use the ability to re-name when exporting (to avoid breaking existing code).
There's no good reason for getting this option multiple times in the same method. Also, we can slightly re-factor how the `editorStampButton` is made visible.
This regressed in PR 16659, when the signature of the `PDFViewer.annotationEditorMode`-setter was changed, and it currently leads to an Error being thrown when exiting PresentationMode.
`PDFViewerApplication` reads from `location.hash` to initialize
`initialBookmark`. But when extensions/chromium/pdfHandler.js prepares
the redirect URL, the reference fragment is encoded instead of bare.
`rewriteUrlClosure` in `chromecom.js` is responsible for decoding the
URL, but that currently runs too late.
To fix this, update `initialBookmark` after rewriting the URL.
This was not a problem in the past because `rewriteUrlClosure` in
`chromecom.js` executed before the initialization of `initialBookmark`.
These options are completely unused in the PDF.js viewer, and given that the last update of the `GrabToPan`-code from upstream was in 2016 it shouldn't hurt to remove them.
