References <https://bugzilla.mozilla.org/show_bug.cgi?id=1879559>
(“In HCM, the text selection is barely visible”).
Continues work from @calixteman who had a partial patch.
This PR improves viewer text-selection highlighting by rendering
selection shapes in the draw layer.
* add selection overlay rendering in the draw layer
* significant code relates to selections spanning multiple text
layers/pages, and edges/end-of-content boundaries
* clear selection on rotate/scale/scroll/spread changes
My main question is: how should it appear?
I don’t have access to the Figma file linked on bugzilla.
In the CSS (`draw_layer-builder.css`) there are 3 blocks:
* default
* `@supports` for browsers supporting `backdrop-filter`
* `forced-colors` mode
So it’s possible to design for those (or more).
Personally, the `backdrop-filter: invert(1)` is the most contrast,
so perhaps it’s better to use something else as the default,
and to use `invert(1)` if high contrast mode is used (maybe with a
`prefers-contrast` media query instead)?
This commit fixes the rendering issue that makes the "must update an
existing annotation" ink editor integration test permafail locally in
Chrome. Note that we already do this for Firefox tests, so this also
improves consistency between the two browsers.
Moreover, improve how we define Chrome options to (similar to their
Firefox counterparts) provide them in a single array, and document the
reasoning for why these options are being set more explicitly.
Fixes#21272.
If the active page is corrupt that currently results in the entire dialog being "blank", thus providing no information, which seems unfortunate and it's easy enough to only skip `pageSizeField` in that rare case.
Improve test coverage for multi-page documents, to ensure that:
- Unnecessary re-parsing is avoided where possible.
- Rotation, in the viewer, is handled correctly.
- Different page sizes are handled correctly.
It looks like this test passes consistently again, most likely after a
combination of browser/Puppeteer/configuration updates and the completed
switch to the WebDriver BiDi protocol.
Fixes#20136.
Originally we introduced a small delay for Puppeteer operations in
Chrome to avoid intermittent failures where protocol calls were
happening too quickly in succession. However, since then a number of
improvements were made, both locally and upstream, that reduce the need
for this delay:
- the integration tests have been hardened to remove (potential) sources
of intermittent failures in many places;
- the browsers and Puppeteer have been updated to improve performance
and support for testing infrastructure;
- the conversion to WebDriver BiDi has been completed, which replaced
the Chrome-specific CDP protocol with a formalized protocol that could
provide more safety guarantees.
This commit therefore reduces the Chrome-specific delay from 5 to 3
milliseconds, which should nowadays be a better value to speed up the
Chrome tests and bring them closer to Firefox in terms of runtime.
The custom solution for obtaining the bounding box of a given element
that we have now was necessary during the original introduction of the
integration tests because at the time the `ElementHandle.boundingBox()`
API in Puppeteer didn't work correctly in Chrome.
However, `getRect`, where this is used, is a hot utility function
because most tests call it multiple times, either directly or indirectly
via other utility functions, and it turns out that the approach we use
is slower than the native `ElementHandle.boundingBox()` API.
Fortunately, most likely after a combination of Chrome/Puppeteer updates
and the conversion to the formalized WebDriver BiDi protocol the custom
solution is no longer necessary because all tests pass without it too,
so this commit converts `getRect` to use `ElementHandle.boundingBox()`
instead to speed up the tests.
These tests could obviously be improved/extended, but it's at least a start to ensure that `ColorConverters` is tested since it's used in both the annotation-layer and the scripting-implementation.
This small helper function only exists to support printing of XFA documents, in the viewer, hence it seems like a good idea to (ever so slightly) reduce the official API surface a little bit.
This is necessary to prevent import cycles with the next patch.
It also shouldn't hurt to reduce the size of `src/display/display_utils.js` a little bit, since utility-files have a tendency to increase in size over time.
Given that this function is only ever used during *parsing* of the PDF document, which happens in the worker-thread, this has always added (a little bit of) dead code in the built `pdf.mjs` file.
Given that the various utility-files naturally increase in size over time, it shouldn't hurt to shorten `src/core/core_utils.js` a little bit by moving a few of its string helper functions to their own file.
This is a new feature in PDF documents, hence it shouldn't hurt to complement the existing ref-test with a simple unit-test as well.
This should also improve test coverage for the `external/` folder, which can't hurt since the other external decoders are already fairly well covered.
Note that for the integration tests the coverage information ends up
being processed in the Node.js context where `window` is not available,
so we use `globalThis` instead for the function that merges individual
test's coverage information into the global object because that is
available in all contexts we support. For clarity we also rename said
function since we're not exclusively dealing with `window` nor worker
data anymore.
It fixes#18032.
Only use the special inner-backdrop compositing path for nested non-isolated groups that actually need isolation.
This preserves the parent/page backdrop for simple nested groups inside knockout groups, preventing later group
compositing from erasing existing backdrop content.
Test PDF files should never be executable because we only read their
contents, so this commit makes sure that all test PDFs have the same
permissions, namely 0644 (read-only for all groups, and writable for the
owner), to limit their permissions for a least-privilege approach.
In a knockout (KO) group each painting operator ("element") composites against
the group's initial backdrop instead of accumulating onto prior elements
of the same group. The backend renders each element to a per-group pooled
temp canvas (keyed off `#groupStackMeta`), builds a binary alpha mask via
a new `feFuncA` filter (`addKnockoutFilter`), `destination-out`s the
group canvas through that mask, restores the initial backdrop into the
cleared footprint for non-isolated groups (cropped to the same mask so
sparse groups don't bleed the whole rectangle), and finally paints the
element on top with the parent's blend mode. Path / clip / transform ops
are mirrored back to the group canvas via `mirrorContextOperations` so
graphics state stays in sync between elements; only the raster pixels
land on the temp canvas.
The temp canvas is forced to source-over for the element raster (`multiply`
on a transparent backdrop would zero the color) and the original GCO is
restored before `copyCtxState` writes back, so the parent's blend mode
survives for the final composite.
Also handled:
- Nested KO groups (the level is incremented for KO, reset to 0 for
non-KO subgroups so an ancestor KO doesn't leak in).
- Non-isolated non-KO subgroups inside a KO parent (`hasInnerBackdrop`
path: blend the elements against the subgroup's running backdrop for
color, mask with the elements-only canvas).
- Soft masks installed inside a KO element (`applySMaskInPlace` in
`compose`, which runs the SMask destination-in directly on the temp
canvas; the existing blit-to-suspended step is gated by `if (!ctx)`).
- Type-3 text, shading fills, image-mask groups, inline images and the
solid-color mask path: each is wrapped in `#begin/#endKnockoutElement`.
- `endDrawing` cleanup so cancelled rendering doesn't leak pooled
canvases or stale knockout state.
This is a major version bump containing two breaking changes for us:
- the `baseUrl` option is removed;
- the `moduleResolution` option doesn't support `node10` (or the `node`
alias) anymore.
The migration guide at https://github.com/microsoft/TypeScript/issues/62508
indicates that we can remove `baseUrl` and change `moduleResolution` to
`bundler` (the latter is consistent with what other projects do that are
linked to the issue, and more details on that configuration option can
be found at https://www.typescriptlang.org/tsconfig/#moduleResolution).
Note that this is enough to get `npx gulp typestest` green and that is
all validation we can do on our side, so as usual if any follow-up fixes
for types are necessary we rely on the community to provide patches and
extend the types test where possible to improve validation.
Given that this function is only ever used in `src/core/` code, let's avoid a little bit of dead code in the *built* `pdf.mjs` file.
Also, place the `AnnotationPrefix` and `AnnotationEditorPrefix` constants together in `src/shared/util.js` since that should aid readability.
Note that `Ref`s and `Name`s are cached globally[1], since that helps reduce object creation (a lot) during parsing.
That cache will be cleared after a period of inactivity in the viewer[2], which is why those primitives cannot *safely* be compared with just `===`/`!==` and also (partially) why abstractions such as `RefSet`/`RefSetCache` are necessary.
Currently `deepCompare` doesn't handle `Ref`s and `Name`s correctly, which may lead to future *intermittent* bugs in any code using the `deepCompare` helper function.
---
[1] This applies to `Cmd` as well, however that doesn't matter in the context of this patch.
[2] Currently, and for more than a decade, set to 30 seconds.
This is a left-over from very old code, which pre-dates the introduction of the `PDFDocumentLoadingTask` and it's nothing more than an alias for its `destroy` method.
Given that `PDFDocumentProxy` already provides a way to access the underlying `PDFDocumentLoadingTask` instance, it shouldn't be necessary to have an alias for one of its methods.
*Please note:* For any existing code relying on the removed method, updating it should be as simple as replacing `pdfDocument.destroy()` with `pdfDocument.loadingTask.destroy()`.
---
[1] If the `PDFDocumentProxy` class was added today, there's no chance that it'd include a `destroy` method.
This is a left-over from very old code[1], before there were a lot of `getDocument` options and when most of the library configuration was done via the (since removed) `PDFJS` global.
Given all the functionality added through the years, which require configuration[2], in practice it's now unlikely that calling `getDocument` without additional options will work except for the most trivial PDFs.
---
[1] If the `getDocument` function was added today, there's no chance that it'd support anything other than a parameter object.
[2] Note things such as CMaps, standard fonts, wasm-based image decoders, and ICC-based colour spaces.
