Currently the `bbox`, `fontMatrix`, and `defaultVMetrics` getters duplicate almost the same code, which we can avoid by adding a new helper method (similar to existing ones for reading numbers and strings).
The added `assert` in the new helper method also caught a bug in how the `defaultVMetrics` length was compiled.
On the worker-thread only the static `write` methods are actually used, and on the main-thread only class instances are being created.
Hence this, after PR 20197, leads to a bunch of dead code in both of the *built* `pdf.mjs` and `pdf.worker.js` files.
This patch reduces the size of the `gulp mozcentral` output by `21 419` bytes, i.e. `21` kilo-bytes, which I believe is way too large of a saving to not do this.
(I can't even remember the last time we managed to reduce build-size this much with a single patch.)
The purpose of PR 11844 was to reduce memory usage once fonts have been attached to the DOM, since the font-data can be quite large in many cases.
Unfortunately the new `clearData` method added in PR 20197 doesn't actually remove *anything*, it just replaces the font-data with zeros which doesn't help when the underlying `ArrayBuffer` itself isn't modified.
The method does include a commented-out `resize` call[1], but uncommenting that just breaks rendering completely.
To address this regression, without having to make large or possibly complex changes, this patch simply changes the `clearData` method to replace the internal buffer/view with its contents *before* the font-data.
While this does lead to a data copy, the size of this data is usually orders of magnitude smaller than the font-data that we're removing.
---
[1] Slightly off-topic, but I don't think that patches should include commented-out code since there's a very real risk that those things never get found/fixed.
At the very least such cases should be clearly marked with `// TODO: ...` comments, and should possibly also have an issue filed about fixing the TODO.
The `PagesMapper` class currently makes up one third of the `src/display/display_utils.js` file size, and since its introduction it's grown (a fair bit) in size.
Note that the intention with files such as `src/display/display_utils.js` was to have somewhere to place functionality too small/simple to deserve its own file.
When recording bboxes for images, it's enough to record their
clip box / bounding box without needing to run the full bbox
tracking of the image's dependencies.
This patch adds right-click support for images in the PDF, allowing
users to download them. To minimize memory consumption, we:
- Do not store the images separately, and instead crop them out of the
PDF page canvas
- Only extract the images when needed (i.e. when the user right-clicks
on them), rather than eagery having all of them available.
To do so, we layer one empty 0x0 canvas per image, stretched to cover
the whole image, and only populate its contents on right click.
These images need to be inside the text layer: they cannot be _behind_
it, otherwise they would be covered by the text layer's container and
not be clickable, and they cannot be in front of it, otherwise they
would make the text spans unselectable.
This feature is managed by a new preference, `imagesRightClickMinSize`:
- when it's set to `-1`, right-click support is disabled
- when set to `0`, all images are available for right click
- when set to a positive integer, only images whose width and height are
greater than or equal to that value (in the PDF page frame of
reference) are available for right click.
This features is disabled by default outside of MOZCENTRAL, as it
significantly degrades the text selection experience in non-Firefox
browsers.
In PR 19548 these checks were added to ensure that the font-data sent from the worker-thread *always* include correct `disableFontFace` and `fontExtraProperties` data.
For some reason PR 20197 then changed the code such that these checks became effectively pointless, since these properties are now checked after the fact *and* the new getters provide fallback values.
This method is only used with loops, and it should be a tiny bit more efficient to use an iterator directly rather than first iterating through the underlying `Map` to create a temporary `Array` that we finally iterate through at the call-site.
The one from pdf.js.utils is a bit too old: a lot of bugs have been fixed
in the code that parses PDF files since then.
It's just an internal development tool, so it doesn't need to be perfect,
but it should be good enough to be useful.
A number of these unit-tests didn't actually cover the intended code-paths, since many of them *accidentally* matched the "file size is smaller than two range requests"-check.
The patch also updates `validateRangeRequestCapabilities` to use return-value names that are consistent with the class fields used in the various stream implementations.
With these changes `0`, `NaN`, `null`, and `undefined` in the `total`-property all result in `percent === NaN` being reported by the callback, since previously e.g. `0` would result in `percent === 100` being reported unconditionally which doesn't make a lot of sense.
Also, remove the "indeterminate" loadingBar (in the viewer) if the `PDFDocumentLoadingTask` fails since there won't be any more data arriving and displaying the animation thus seems wrong.
The user has to select some pages and then click on the "Save As" menu item in the Manage menu.
If they modify the structure of the pdf (deleted, moved, copied pages), they have to use the usual
save button.
This solves [bug 1989406](https://bugzilla.mozilla.org/show_bug.cgi?id=1989406).
(“The user should be able to dismiss the in-content message displayed by clicking somewhere else in the PDF”)
There’s a good gif there that shows the problematic behavior.
In the thread, there are also mentions of 2 similar but slightly separate problems:
* clicking on another highlight should also dismiss
* the mention that hitting the escape key does not dismiss
I found the last point, the escape key, to work already (first test case here).
But this PR solves the main bug (second test case) and the adjacent one
(third test case).
It works by using the existing `unselectAll` handling.
This will ignore filenames that become effectively empty, i.e. ones that are only ".pdf" and nothing more.
*Please note:* While this passes all existing unit-tests, I don't know if this is necessarily the "correct" solution here.
Currently the transfers aren't actually being used with the "GetOperatorList" message, since the placement of the parameter is wrong; note the method signature: 909a700afa/src/shared/message_handler.js (L219-L229)
This goes back to PR 16588, which added the transfers parameter, and unfortunately we all missed that :-(
Simply fixing the parameter isn't enough however, since that broke printing of Stamp-editors (and possibly others). The solution here is to *not* transfer data during printing, given that a single `PrintAnnotationStorage` instance is being used for all pages.
Change all these cases to use `Map.prototype.getOrInsertComputed()` instead, in combination with a helper function for creating the `Array`s (similar to the previous patch).
With the exception of the first invocation the callback function is unused, which means that a lot of pointless functions may be created.
To avoid this we introduce helper functions for simple cases, such as creating `Map`s and `Objects`s.
Currently we essentially "duplicate" the same code for parsing the `values` and `keys` of the `searchParams`, which seems a little unnecessary.
To be able to parse the `searchParams` from the end, we currently create an Array (from the Iterator) and then reverse it before finally looping through it. Here the latter two steps can be replaced with the `Array.prototype.findLast()` method instead.
*Please note:* I completely understand if this patch is rejected, on account of being less readable than the current code.
This leads to slightly shorter code[1] when initializing classes, and in some cases we can even remove the constructors, which shouldn't hurt; see https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/prefer-class-fields.md
It's probably possible to also change a lot of these class fields to private ones[2], however it's often difficult to tell at a glance if that's safe hence this patch only does this for the `PDFRenderingQueue`.
---
[1] This reduces the size of the `gulp mozcentral` output by 999 bytes, for a mostly mechanical code change.
[2] That sort of re-factoring should generally be done separately, on a class-by-class basis, to reduce the risk of regressions.
In all cases where we currently use `Response.prototype.arrayBuffer()` the result is immediately wrapped in a `Uint8Array`, which can be avoided by instead using the newer `Response.prototype.bytes()` method; see https://developer.mozilla.org/en-US/docs/Web/API/Response/bytes
Given that the Node.js code uses standard `ReadableStream`s now, see PR 20594, it can use the same `getArrayBuffer` as the Fetch API implementation.
Also, change the `getArrayBuffer` fallback case to an Error (rather than a warning) since that should never actually happen.
