We have a fallback for the common case of Type3 fonts without a /FontDescriptor dictionary, however we also need to handle the case where it's present but lacking the required /FontName entry.
The `Page.evaluate()` and `Mouse.click()` APIs in Puppeteer both return
a promise; see https://pptr.dev/api/puppeteer.page.evaluate and
https://pptr.dev/api/puppeteer.mouse.click, and should therefore be
awaited before proceeding in tests to make sure that the test's behavior
is deterministic and avoid intermittent failures.
The following command was used to find potential places to fix:
`grep -nEr "[^await|return] page\." test/integration/*`
The clipboard, used via the `copyImage` helper function, is a shared
resource, so access to it cannot happen concurrently because it could
result in tests overwriting each other's contents. Most tests using
the clipboard are therefore run sequentially, but only the stamp
editor's undo-related tests weren't, so this commit fixes the issue
by running those tests sequentially too.
This allows us to simply invoke `PDFWorker.create` unconditionally from the `getDocument` function, without having to manually check if a global `workerPort` is available first.
This way it helps to reduce the overall canvas dimensions and make the rendering faster.
The drawback is that when scrolling, the page can be blurry in waiting for the rendering.
The default value is 200% on desktop and will be 100% for GeckoView.
Currently *some* of the links[1] on page three of the `issue19835.pdf` test-case aren't clickable, since the destination (of the LinkAnnotation) becomes empty.
The reason is that these destinations include the character `\x1b`, which is interpreted as the start of a Unicode escape sequence specifying the language of the string; please refer to section [7.9.2.2 Text String Type](https://opensource.adobe.com/dc-acrobat-sdk-docs/pdfstandards/PDF32000_2008.pdf#G6.1957385) in the PDF specification.
Hence it seems that we need a way to optionally disable that behaviour, to avoid a "badly" formatted string from becoming empty (or truncated), at least for cases where we are:
- Parsing named destinations[2] and URLs.
- Handling "strings" that are actually /Name-instances.
- Building a lookup Object/Map based on some PDF data-structure.
*NOTE:* The issue that prompted this patch is obviously related to destinations, however I've gone through the `src/core/` folder and updated various other `stringToPDFString` call-sites that (directly or indirectly) fit the categories listed above.
---
[1] Try clicking on anything on the line containing "Item 7A. Quantitative and Qualitative Disclosures About Market Risk 27".
[2] Unfortunately just skipping `stringToPDFString` in this case would cause other issues, such as the named destination becoming "unusable" in the viewer; see e.g. issues 14847 and 14864.
This integration test fails intermittently because the undo button can
only be activated if focus can be put on it, and that in turn can only
happen if it's visible. The test tried to make sure that the undo bar
is visible, but checking for the absence of the `hidden` attribute is
unfortunately not enough to assert visibility according to Puppeteer
documentation [1]. Moreover, the undo button wasn't checked at all.
To fix the issue we let Puppeteer do the visibility detection for the
undo bar by providing the `visible: true` option to `waitForSelector`
[2]. This is consistent with the other tests that already do this, and
also with the existing code that detects if the undo bar is hidden
(which uses the `hidden: true` option of `waitForSelector`). Moreover,
we wait for the undo button to be present before putting focus on it.
For consistency, and to avoid intermittent failures elsewhere, we mirror
this solution to the other undo bar/button tests of the various editors.
[1] https://pptr.dev/api/puppeteer.elementhandle.isvisible
[2] https://pptr.dev/api/puppeteer.waitforselectoroptions
In the referenced PDF document the keys, in the /Dests dictionary, need to account for PDFDocEncoding.
To improve destination handling in general we'll now unconditionally fallback to always checking all destinations.
This is a temporary measure to reduce noise until #19811 is fixed. Note
that this shouldn't be an issue in terms of coverage because we still
run the test in Firefox.
In Webpack version `5.99.0` the way that `export` statements are handled was changed slightly, with much less boilerplate code being generated, which unfortunately breaks our `tweakWebpackOutput` function that's used to expose the exported properties globally and that e.g. the viewer depends upon.
Given that we were depending on formatting that should most likely be viewed as nothing more than an internal implementation detail in Webpack, we instead work-around this by manually defining the structures that were previously generated.
Obviously this will lead to a tiny bit more manual work in the future, however we don't change the API-surface often enough that it should be a big issue *and* the relevant unit-tests are updated such that it shouldn't be possible to break this.
*NOTE:* In the future we might want to consider no longer using global properties like this, and instead rely only on proper `export`s throughout the code-base.
However changing this would likely be non-trivial (given edge-cases), and it'd be an `api-major` change, so let's just do the minimal amount of work to unblock Webpack updates for now.
It's no longer necessary after commit 1c73e52 that caused the document
to be closed properly between tests, and this therefore partly reverts
commit 973b67f.
This commit configures Jasmine to no longer run the tests in a fixed
order, which combined with the previous isolation commits avoids being
able to accidentally introduce dependencies between integration tests.
To avoid being able to introduce dependencies between tests this commit
makes sure that we close the document between tests so that we can't
accidentally rely on state set by a previous test.
To avoid being able to introduce dependencies between tests this commit
makes sure that we close the document between tests so that we can't
accidentally rely on state set by a previous test.
To avoid being able to introduce dependencies between tests this commit
makes sure that we close the document between tests so that we can't
accidentally rely on state set by a previous test.
To avoid being able to introduce dependencies between tests this commit
makes sure that we close the document between tests so that we can't
accidentally rely on state set by a previous test.
To avoid being able to introduce dependencies between tests this commit
makes sure that we close the document between tests so that we can't
accidentally rely on state set by a previous test.
To avoid being able to introduce dependencies between tests this commit
makes sure that we close the document between tests so that we can't
accidentally rely on state set by a previous test.
To avoid being able to introduce dependencies between tests this commit
makes sure that we close the document between tests so that we can't
accidentally rely on state set by a previous test.
To avoid being able to introduce dependencies between tests this commit
makes sure that we close the document between tests so that we can't
accidentally rely on state set by a previous test.
To avoid being able to introduce dependencies between tests this commit
makes sure that we close the document between tests so that we can't
accidentally rely on state set by a previous test.
To avoid being able to introduce dependencies between tests this commit
makes sure that we close the document between tests so that we can't
accidentally rely on state set by a previous test.
To avoid being able to introduce dependencies between tests this commit
makes sure that we close the document between tests so that we can't
accidentally rely on state set by a previous test.
To avoid being able to introduce dependencies between tests this commit
makes sure that we close the document between tests so that we can't
accidentally rely on state set by a previous test.
For simplicity we will abort /Form XObject parsing *immediately* when encountering a circular reference, rather than letting it continue up until some limit (as e.g. PDFium appears to do), which should be fine since there are never any guarantees if/how *corrupt* PDF documents will render.
Previously we'd only do this for Type1/CFF fonts, see e.g. PR 6736, since the font-program may update the /FontMatrix.
However, it seems that we should do this unconditionally to account for fonts with non-default /FontMatrix-entries in the font-dictionary (which seem to be pretty rare).
In PDF version 2.0 the handling of Indexed color spaces was clarified as follows:
> The index value should be an integer in the range 0 to hival. If the value is a real number, it shall be rounded to the nearest integer (0.5 values shall be rounded up); if it is outside the range 0 to hival, it shall be adjusted to the nearest value within that range.
Please refer to https://github.com/pdf-association/pdf-differences/tree/main/IndexedColor
The current text layer approach based on absolutely positioned
`<span>` elements by default causes flickering with text selection,
and we have browser-specific workarounds to solve that.
In Chrome, the workaround involves moving the `.endOfContent` element to
right after the last element that contains some selected content. This
works well in simple PDFs, but breaks when we have `span.markedContent`
elements. Given a text layer structure like the following, rendered
as four consecutive lines:
```html
<span class="markedContent">
<br>
<span>development enter the construction phase (estimated at around</span>
</span>
<span class="markedContent">
<br>
<span>300 MEUR).</span>
</span>
<span class="markedContent">
<br>
<span>Kreate's EBITA increased to 2.8 MEUR (Q4'23: 2.7 MEUR) and the</span>
</span>
<span class="markedContent">
<br>
<span>margin rose to 3.7% (Q4'23: 3.4%). However, profitability was</span>
</span>
```
when starting to select from inside the first line and dragging down
to the empty space after the second line, Chrome will anchor the
selection at the beginning of either the `<br>` or the `<span>` inside
the last `.markedContent`, depending on whether the selection is in
"per-character mode" (i.e. click and drag) or "per-word mode" (i.e.
double click and drag). This causes us to insert the `.endOfContent`
element in the wrong place (one element too far), which causes one
more line to be selected, which triggers another `"selecctionchange"`
event, which causes us to move `.endOfContent` again, and so on, looping
until when the whole page is selected.
This commit fixes the issue by making sure that when the end of the
selection range points to the _begining_ of an element, we walk back
the dom finding the first non-empty element, and attatch `.endOfContent`
to the end of that.
These `getAll` methods are not used anywhere within the PDF.js code-base, outside of tests, and were mostly added (speculatively) for third-party users.
To still allow access to the same data we instead introduce iterators on these classes, which (slightly) shortens the code and allows us to remove the `objectFromMap` helper function.
A summary of the changes in this patch:
- Replace the `getAll` methods with iterators in the following classes: `AnnotationStorage`, `Metadata`, and `OptionalContentGroup`.
- Change, and also re-name, `AnnotationStorage.prototype.setAll` into a test-only method since it's not used elsewhere.
- Remove the `Metadata.prototype.has` method, since it's only used in tests and can be trivially replaced by calling `Metadata.prototype.get` and checking if the returned value is `null`.
We want to iterate through the data in the `computeMD5` function, and `Map`s have "nicer" support for that than generic objects.
(Somewhat recently `Map` performance was improved in Firefox, however this also isn't really performance sensitive code.)
