Currently this `assert` isn't actually doing what it's supposed to, since the `FontLoader`-class doesn't have a `disableFontFace`-field.
The `FontFaceObject`-class on the other hand has such a field, hence we update the method-signature to be able to check the intended thing.
Web archive no longer has the revision for the saved PDF referenced in
that test case, so this updates that link to a more recent revision to
enable the tests to run on a clean clone.
The idea is to avoid to have the pasted editor hidding the copied one:
the user could think that nothing happened.
So the top-left corner of the pasted one is moved to the bottom-right corner of the copied one.
This replaces the various copies of this logic with a single helper that
we template for each editor type, similar to what we already do for the
`switchToEditor` helper.
This replaces the various copies of this logic with a single helper that
we template for each editor type, similar to what we already do for the
`switchToEditor` helper.
When a drawing was moved with arrow keys and then printed or saved, the drawing wasn't moved finally.
So the fix is just about calling onTranslated once the translation is done.
With the recently added OpenJPEG no-wasm fallback we need to send the `wasmUrl` option to the worker-thread *regardless* of the value of the `useWorkerFetch` option, since the fallback won't work if we don't have a URL to `import` it from.
For consistency the code is re-factored to always send the factory-urls to the worker-thread, and simply check the `useWorkerFetch` option there instead.
Also, as a follow-up to PR 19525, introduce a new `useWasm` option that can be used in e.g. browser-tests to forcibly disable WebAssembly usage.
This patch fixes an issue when pasting: an exception was thrown when pasting.
And while writing the test and comparing the paths in the svg, I found a difference
which is fixed thanks to call to the right constructor (to take into account the inheritance)
in inkdraw.js
When scrolling quickly, the constant re-rendering of the detail view
significantly affects rendering performance, causing Firefox to
not render even the _background canvas_, which is just a static canvas
not being re-drawn by JavaScript.
This commit changes the viewer to only render the detail view while
scrolling if its rendering hasn't just been cancelled. This means that:
- when the user is scrolling slowly, we have enough time to render the
detail view before that we need to change its area, so the user always
sees the full screen as high resolution.
- when the user is scrolling quickly, as soon as we have to cancel a
rendering we just give up, and the user will see the lower resolution
canvas. When then the user stops scrolling, we render the detail view
for the new visible area.
When rendering big PDF pages at high zoom levels, we currently fall back
to CSS zoom to avoid rendering canvases with too many pixels. This
causes zoomed in PDF to look blurry, and the text to be potentially
unreadable.
This commit adds support for rendering _part_ of a page (called
`PDFPageDetailView` in the code), so that we can render portion of a
page in a smaller canvas without hiting the maximun canvas size limit.
Specifically, we render an area of that page that is slightly larger
than the area that is visible on the screen (100% larger in each
direction, unless we have to limit it due to the maximum canvas size).
As the user scrolls around the page, we re-render a new area centered
around what is currently visible.
This base class contains the generic logic for:
- Creating a canvas and showing when appropriate
- Rendering in the canvas
- Keeping track of the rendering state
Those type tests are performing type checking on a project using DOM APIs, intended to reflect the usage in a non-node project.
Not loading the node types in that project ensures that the library type declarations don't force a dependency on the node types.
Currently we modify the EXIF-block in place, which may end up "breaking" the JPEG-data of the original PDF document since e.g. saving it from the viewer no longer contains the real EXIF-block.
Hence the EXIF-block replacement is moved into the `JpegStream` class, such that we can copy the data before doing the replacement.
During the XRef stream parsing we're attempting to lookup an entry that hasn't yet been found, since parsing is currently running, and given that we'd also cache free/missing XRef entries we'd then return an incorrect value during normal PDF parsing.
The simplest solution here is to just not cache free/missing XRef entries, since a properly generated PDF document shouldn't be trying to access objects it doesn't contain.
Furthermore, the amount of "extra" parsing now needed for such XRef entries shouldn't be significant enough to be an issue.
It fixes#19505.
We were invaliding throwing actions (in setting event.rc to false) and all the event process was stopped.
Now we're just dumping the exception in the console: the action is skipped and event.rc is not set
else the input fields aren't updated wit KeyStroke actions.
Currently this rule is disabled in a number of spots across the code-base, and unless absolutely necessary we probably shouldn't disable linting, so let's just update the code to fix all the outstanding cases.
If either of the factory-urls are missing or invalid, the fallback value would currently become `useWorkerFetch === null`.
While that is obviously a falsy value, which means that the code still works as intended, we should ensure that this is consistent.
- Use `TypedArray.prototype.set()` rather than a manual loop when building the `key`.
- Use an existing local variable to avoid re-computing the length of the `encryptionKey`.
It's now pretty common that we only want to close a `dialog` *if* it's currently active, to avoid throwing errors, and this new method provides a shorter and more convenient way to achieve that.
Rather than modifying the "raw" dimensions of the page, we'll instead apply the `userUnit` as an *additional* scale-factor via CSS.
*Please note:* It's not clear to me if this solution is fully correct either, or if there's other problems with it, but it at least *appears* to work.
---
With these changes, the following CSS variables are now assumed to be available/set as necessary: `--total-scale-factor`, `--scale-factor`, `--user-unit`, `--scale-round-x`, and `--scale-round-y`.
While investigating a bug, that I've not yet had time to fully investigate and report, I found that if there's ever an error thrown from the `Autolinker` class it'll prevent the annotationEditorLayer from rendering *and* the renderTask itself will be treated as having failed.
Given that most inferred links will overlap existing LinkAnnotations, creating a lot of unused `borderStyle` objects seem unnecessary.
Hence we can move that into the `AnnotationLayer.prototype.addLinkAnnotations` method instead, which also allows us to slightly reduce the API-surface.
*Note:* For the issue mentioned on Matrix it'll obviously still make sense to improve the regular expression to detect more URL edge-cases.
However it occurred to me that even once that particular case is fixed there'll always be a risk that inferred links could overlap, and effectively block, the actual LinkAnnotations.
Hence this patch removes the URL comparison to ensure that overlapping inferred links will always be ignored.
To avoid being able to introduce dependencies between tests, and to
bring existing dependencies to the surface, 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. This prevents multiple tests from
failing if one of them fails and makes debugging easier by being able to
run each test on their own independent of other tests.
This commit, combined with the previous one, is enough to make the ink
editor integration test suite pass consistently if random mode in
Jasmine is enabled, proving that the tests are fully isolated now.
The second test of the basic operations block for the ink editor
depends on the first test to work. This becomes visible if we only run
the second test, using `fit`, which always fails with:
`ProtocolError: Waiting for selector '.annotationEditorLayer' failed:
Runtime.callFunctionOn timed out. Increase the 'protocolTimeout' setting
in launch/connect calls for a higher timeout if needed.`
The problem is that the second test doesn't enable the ink editor and
relies on the first test having done that already (because we don't
close the document between tests yet). This commit fixes the issue by
unconditionally enabling the ink editor in the second test to remove the
dependency between the two tests so they both pass in isolation.
This patch updates the minimum supported browsers as follows:
- Google Chrome 110, which was released on 2023-02-07; see https://chromereleases.googleblog.com/2023/02/stable-channel-update-for-desktop.html
Note that nowadays we usually try, where feasible and possible, to support browsers that are about two years old. By limiting support to only "recent" browsers we reduce the risk of holding back improvements of the *built-in* Firefox PDF Viewer, and also (significantly) reduce the maintenance/support burden for the PDF.js contributors.
*Please note:* As always, the minimum supported browser version assumes that a `legacy`-build of the PDF.js library is being used; see https://github.com/mozilla/pdf.js/wiki/Frequently-Asked-Questions#faq-support
Currently we have three separate and virtually identical message handlers for this data, which can easily be combined into a single message handler instead.
Automatically detect links in the text content of a file and automatically
generate link annotations at the appropriate locations to achieve
automatic link detection and hyperlinking.
With recent changed made to the GitHub issues-UI the "Blank issue" alternative is now showing up quite prominently, which can easily negate the point of our bug/feature templates and lead to incomplete issues being filed.
Given that we now have a few different factory-url parameters, we introduce a helper function for parsing them.
*Please note:* These parameters have always been documented as requiring a trailing slash[1], which we can now easily enforce during the `getDocument`-call.
---
[1] I recall that we've occasionally seen issues because users miss that detail, and the new Error should hopefully be more easily actionable than one thrown during rendering/parsing.
This pattern was already followed quite consistently outside of the
freetext editor integration tests, so this commit aligns the remaining
places for consistency. This also helps to make the tests more compact
and to reduce the number of changes in follow-up changes.
In most integration tests we already use the pattern of defining the
editor selector once and reusing it in the rest of the test, but it's
not fully consistent everywhere yet. This commit fixes that for the
freetext editor integration tests, which has multiple advantages:
- it improves consistency between the various editor integration tests;
- it removes duplicate function calls and aligns variable definitions;
- it reduces the number of `getEditorSelector` calls that contained
hardcoded IDs, which helps to isolate the tests and to simplify
follow-up patches.
This CSS feature is now available in *most* browsers that we support, with old Chromium-based browsers being the only exception; please see https://developer.mozilla.org/en-US/docs/Web/CSS/color_value/color-mix#browser_compatibility
From this data we see that the feature in question has been supported since Chrome 111, which was released on 2023-03-01 (i.e. almost two years ago).
Please note that we've never guaranteed that all features and functionality will be available in the oldest supported browsers.
Furthermore, even with the `color-mix` fallback removed PopupAnnotations will still function just as before but may render with the default color (defined in the CSS-file) rather than the one specified in the PDF document.
In most integration tests we already use the pattern of defining the
editor selector once and reusing it in the rest of the test, but it's
not fully consistent everywhere yet. This commit fixes that for the
highlight editor integration tests, which has multiple advantages:
- it improves consistency between the various editor integration tests;
- it removes duplicate function calls and aligns variable definitions;
- it reduces the number of `getEditorSelector` calls that contained
hardcoded IDs, which helps to isolate the tests and to simplify
follow-up patches.
At the time of PR 12896 the `fontBoundingBox{Ascent, Descent}` properties were not yet available by default in Fírefox, however that's no longer the case since Firefox 116; please see https://bugzilla.mozilla.org/show_bug.cgi?id=1801198.
Hence this patch which replaces the "full" fallback with a warning and uses the `ascent`/`descent` values from the fonts in the PDF document (as we did previously). Obviously the TextLayer won't look as good in that case, but it's a simpler and shorter solution.
Currently we're manually computing the width/height of the /Rect-entry in a number of spots throughout the worker-thread Annotation code, which these new getters help avoid.
Currently we're initializing the image-options for every page, which seems unnecessary since it should suffice to do that once per document.
Also, changes the `BasePdfManager` constructor to improve readability/documentation a little bit.
This patch is adding some code in order to extract a drawing as curves from an image.
The algorithm is basically the following:
- reduce the dimensions
- make it gray
- apply a bilateral filter in order to add some blurryness while keeping the edges
- compute the histogram
- guess what's the background color which should contain a large majority of the pixels
- make a binary image
- extract the contours in using the Suzuki algorithm
- apply the Douglas-Peucker algorithm in order to reduce the number of points
The algorithm is improvable but it should work pretty well if there's a clear difference between
the background and the drawing.
In a v2 we could use a ML model in order to improve the extraction.
There's few changes related to the UI in order to make the tool usable, but they're very basic
for the moment.
This patch fixes a bug that caused incorrect curve shapes when an endpoint lies beyond the page boundaries. It adds a check for the endpoint's position, and if it is outside the page, the point is excluded from the shape's coordinates.
Steps to reproduce this in `master`:
1. Open https://mozilla.github.io/pdf.js/web/viewer.html
2. Use the "Open"-button (in the secondaryToolbar), or drag-and-drop, to load another PDF document.
3. Enable the highlight-editor.
4. Try to pick a new colour.
Note how it's no longer possible to change the default highlight-colour.
The reason for this is that we're only initializing the viewer-toolbar `ColorPicker` *once*, which doesn't work since every PDF document gets its own `AnnotationEditorUIManager`-instance. To address this we simply need to re-initialize the viewer-toolbar `ColorPicker`, and note that this patch won't affect the Firefox PDF Viewer.
With the changes in PR 18843 the `AnnotationEditorUIManager.prototype.updateMode` method is now asynchronous, which we need to take into account when dispatching the "annotationeditormodechanged" event.
After PR 2317, which landed in 2012, we'd immediately clean-up after rendering for pages with large image resources. This had the effect that re-rendering, e.g. after zooming, would force us to re-parse the entire page which could easily lead to bad performance.
In PR 16108, which landed in 2023, we tried to lessen the impact of that by slightly delaying clean-up however that's obviously not a perfect solution (and it increased the complexity of the relevant code).
Furthermore, the condition for this "immediate" clean-up seems a bit arbitrary to me since a page could easily contain a large number of smaller images whose total size vastly exceeds the threshold.
Hence this patch, which suggests that we remove the conditional and delayed clean-up after rendering. Compared to the situation back in 2012, a number of things have improved since:
- We have *multiple* caches for repeated image-resources on the worker-thread[1], which helps reduce overall memory usage and improves performance.
- We downsize huge images on the worker-thread, which means that the images we're using on the main-thread cannot be arbitrarily large.
- The amount of available RAM on devices should be a lot higher, since more than a decade has passed.
A future improvement here, for more resource constrained environments, could be to instead clean-up when actually needed using e.g. `WeakRef`s (see issue 18148).
---
[1] More specifically:
- `LocalImageCache`, which caches image-data by /Name and /Ref on the `PartialEvaluator.prototype.getOperatorList` level.
- `RegionalImageCache`, which caches image-data by /Ref on the `PartialEvaluator`-instance (i.e. at the page) level.
- `GlobalImageCache`, which caches image-data by /Ref globally at the document level.
It fixes#19360.
Each glyph in the test case has a fill and a stroke pattern, so the current transform used
to scale the glyph outline must be the same.
In setting the stroke color to green, I noticed that the last outline contains some non-closed
subpaths, so when generating the glyph outline, every time we 'moveTo', we close the previous
subpath.
Code in the `web/` folder cannot import directly from the `src/` folder, since that could result in most (or all) main-thread code being bundled into the viewer, and must rather be imported via the `pdfjs-lib` alias.
Let's use ESLint to help enforce this, please find additional details in https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/no-restricted-paths.md
In most integration tests we already use the pattern of defining the
editor selector once and reusing it in the rest of the test, but it's
not fully consistent everywhere yet. This commit fixes that for the
ink editor integration tests, which has multiple advantages:
- it improves consistency between the various editor integration tests;
- it removes duplicate function calls and aligns variable definitions;
- it reduces the number of `getEditorSelector` calls that contained
hardcoded IDs, which helps to isolate the tests and to simplify
follow-up patches.
In most integration tests we already use the pattern of defining the
editor selector once and reusing it in the rest of the test, but it's
not fully consistent everywhere yet. This commit fixes that for the
stamp editor integration tests, which has multiple advantages:
- it improves consistency between the various editor integration tests;
- it removes duplicate function calls and aligns variable definitions;
- it reduces the number of `getEditorSelector` calls and other helper
function calls that contained hardcoded IDs (by updating them to take
editor selectors as arguments instead of editor IDs), which helps to
isolate the tests and to simplify follow-up patches.
Puppeteer recently got updated to version 24.0.0+, so we're past version
23.9.1- where the integration test failed before and we can enable it
again now that it passes in Chrome.
This has multiple advantages:
- it improves consistency between the various editor integration tests;
- it makes the code easier to read/understand;
- it reduces code duplication.
This has multiple advantages:
- it improves consistency between the various editor integration tests;
- it makes the code easier to read/understand;
- it reduces code duplication;
- it reduces the number of `getEditorSelector` calls that contained
hardcoded IDs, which helps to isolate the tests and to simplify
follow-up patches.
The `selectorEditor` name is used 57 times, but only in the freetext
editor integration tests file. The `editorSelector` name on the other
hand is used 241 times in all editor integration test files, so this
commit renames the former name to the latter name to achieve consistency
in variable names across all editor integration test files, which also
simplifies upcoming changes.
- Most of the these are only used in the `src/display/api.js` file, and this leads to slightly shorter code.
- A number of unit-tests need a `BaseCanvasFactory`-instance, however that one is available through the `PDFDocumentProxy`-instance nowadays.
- For other unit-tests the remaining necessary default Factory-definitions can be moved into the `test/unit/test_utils.js` file.
This prepares for a future where we're using more than one wasm-file, originating in different `external/`-folders, by extending the existing `gulp.watch` usage.
The following diff illustrates how to add more entries:
```diff
diff --git a/gulpfile.mjs b/gulpfile.mjs
index 0e0a5a1ac..1502755be 100644
--- a/gulpfile.mjs
+++ b/gulpfile.mjs
@@ -655,6 +655,10 @@ function createWasmBundle() {
base: "external/openjpeg",
encoding: false,
}),
+ gulp.src(["external/foobar/*.wasm"], {
+ base: "external/foobar",
+ encoding: false,
+ }),
]);
}
@@ -2125,7 +2129,7 @@ gulp.task(
},
function watchWasm() {
gulp.watch(
- "external/openjpeg/*",
+ ["external/openjpeg/*", "external/foobar/*"],
{ ignoreInitial: false },
gulp.series("dev-wasm")
);
```
Rather than waiting for the upstream patch to reach the Firefox version we're using with Puppeteer, let's just set the same preference as done in https://phabricator.services.mozilla.com/D234320.
Currently we're not checking that the response is actually OK before getting the data, which means that rather than throwing an error we can get an empty `ArrayBuffer`.
To avoid duplicating code we can move an existing helper into `src/core/core_utils.js` and re-use it when fetching the JPX wasm-file as well.
Given that we nowadays provide default Node.js versions of these Factory-parameters it no longer seems necessary to mention that environment specifically.
These old exceptions have a fair amount of overlap given how/where they are being used, which is likely because they were introduced at different points in time, hence we can shorten and simplify the code by replacing them with a more general `ResponseException` instead.
Besides an error message, the new `ResponseException` instances also include:
- A numeric `status` field containing the server response status, similar to the old `UnexpectedResponseException`.
- A boolean `missing` field, to allow easily detecting the situations where `MissingPDFException` was previously thrown.
In order to fix bug 1935076, we'll have to add a pure js fallback in case wasm is disabled
or simd isn't supported. Unfortunately, this fallback will take some space.
So, the main goal of this patch is to reduce the overall size (by ~93k).
As a side effect, it should make easier to use an other wasm file (which must export
_jp2_decode, _malloc and _free).
When a dash separates two digits, it's very likely to not be a hyphen
inserted to split a word into two lines (e.g. "par\n-ser"), but rather
either a minus sign, a range, or a date. For example, in the tracemonkey
PDF there is `2008-02` (a date) split across two lines.
Preserving the dash, similarly to how we do for compound words, allows
searches for "2008-02" to find a match.
In Firefox, double-clicking on a stamp annotation triggers text
selection (selecting the last text element in the dom before the
annotation): this triggers the logic to make annotations not interfere
with text selection, which in turns prevents the double click from
triggering the annotation editor.
This commit fixes the problem by making annotations non-selectable, so
that clicking on them does not trigger text selection. Freetext
annotations were already non-selectable, so this commit doesn't change
that. However, we need to explicitly mark text in popups as selectable.
In the affected font the total number of mapping-entries is `1142348`, and no less than `997473` of them are duplicates.
Given that every duplicate causes a lot of Array elements to be moved this becomes extremely inefficient, which we can avoid by keeping track of seen `charCode`s and directly build the final mappings-Array instead.
Currently we re-implement a number of helper functions specifically for this code, which seems completely unnecessary since there's already general purpose ones available in the `src/core/core_utils.js` file.
It lets the user make a pinch gesture with a finger on page with a drawing
and the second finger on an other page.
On mobile, it's pretty easy to be in such a situation.
It fixes#19239.
When the canvas isn't existing the editor has no image: it's fine because the editor is invisible.
Once it's made visible, the canvas is set when the annotation layer has been rendered.
This appears to have regressed in PR 13808, since it removed the `matrix`-entry from array returned by the `MeshShading.prototype.getIR` method *without* also updating the indexes in the `MeshShadingPattern` constructor.
Reasons for removal:
- These tests never generated any warnings from OSS-Fuzz, in over a year.
- An error thrown during image decoding will lead to a broken/missing image, not a security problem.
- These tests rely on the Jazzer.js library, which has a number of problems: It now causes failures in Node.js v23 in the CI tests, it's no longer being maintained upstream, and it lacks support for some (fairly common) CPU architectures.
The ink editor integration tests already use a helper function for
committing the editor, so this commit mirrors the approach to the
freetext editor integration tests. This has multiple advantages:
- it improves consistency between the various editor integration tests;
- it makes the code easier to read/understand;
- it reduces code duplication (220 lines of code removed);
- it reduces the number of `getEditorSelector` calls (32 calls removed)
that contained hardcoded IDs, which helps to isolate the tests and to
simplify follow-up patches.
This commit applies the `waitForUnselectedEditor` helper function to the
remaining places, that most likely predate the introduction of the
helper function, to deduplicate the code and to have a unified way of
checking if a given editor is unselected.
The `/Root/AcroForm/Fields` array contains a "ridiculous" number of LinkAnnotations, which obviously makes no sense since those are not form fields.
To improve performance we'll thus ignore those when collecting the field objects.
There's a few cases where we're looping through the result of `Dict.prototype.getKeys` and then manually look-up the values, which after PR 19051 can be replaced with direct iteration instead.
The `getSelectedEditors` function is largely a copy of the `getEditors`
function, with three small differences:
1. `getEditors` allows getting any kind of editor whereas
`getSelectedEditors` is harcoded to only getting selected editors.
2. `getEditors` returns editor selectors (strings) whereas
`getSelectedEditors` returns editor IDs (integers).
3. `getSelectedEditors` returns a sorted array of editor IDs whereas
`getEditors` does not ensure that the array is sorted.
This commit makes the `getEditors` function a drop-in replacement for
the `getSelectedEditors` function to deduplicate the code and to have a
unified way of getting editors.
Note that we don't actually use the contents of the returned array
(only its length), so we can safely change `getEditors` to return a
sorted array of integer editor IDs instead. Sorting the array makes the
return value deterministic, which is a nice property for test stability,
and integer IDs are also easier to handle in test assertions. Note that
the corresponding selector strings can also easily be obtained from the
integer IDs using the `getEditorSelector` function if needed.
Originally the code in this file was used in both the GENERIC and MOZCENTRAL builds, however that's no longer the case.
Hence we can now directly call `NetworkManager.prototype.request` and remove these old "helper" methods that only had a single call-site each.
From section [11.6.4.3 Mask Shape and Opacity](https://opensource.adobe.com/dc-acrobat-sdk-docs/pdfstandards/PDF32000_2008.pdf#G10.4848628) in the PDF specification:
- An image XObject may contain its own *soft-mask image* in the form of a subsidiary image XObject in the `SMask` entry of the image dictionary (see "Image Dictionaries"). This mask, if present, shall override any explicit or colour key mask specified by the image dictionary's `Mask` entry. Either form of mask in the image dictionary shall override the current soft mask in the graphics state.
As part of the changes in PR 4259, which landed over ten years ago, the `glyphNameMap` property on `Font`-instances was removed.
The reason that this didn't cause any bugs is that we always fallback on `getGlyphsUnicode`, and when using that data we also rely on `StandardEncoding`, hence we should just remove the unused parameter from the `Type2Compiled` constructor.
While [bug 1893645](https://bugzilla.mozilla.org/show_bug.cgi?id=1893645) was fixed some time ago now, it still shouldn't hurt to also assert that the `fontMatrix` is always valid when invoking the `compileGlyph` method.
Rather than having to manually implement the exception-handling for the "DocException" message, we can instead re-use (and slightly extend) the existing `wrapReason` function since that one already does what we need.
Furthermore, we can also simplify handling of the "PasswordRequest" message a little bit and again re-use the `wrapReason` function.
Finally, the patch makes the following smaller changes:
- Avoid needlessly re-creating exceptions in the `wrapReason` function.
- Use a slightly shorter parameter name in the `wrapReason` function.
- Remove the unused entries in the `CallbackKind`/`StreamKind` enumerations.
This commit makes sure that the font tests are no longer reported as
being unit tests and that the PDF.js logo is shown in the browser tab to
make PDF.js-specific resources/tabs more easily identifyable during e.g.
development.
The reporter is used in both the unit and the font tests, so this commit
moves it to the test root folder to more clearly indicate that this is a
shared resource and so the font tests don't have to reach into the unit
tests folder to import it (which improves separation).
This file is specific to the integration tests, so this commit moves it
to bundle the integration test logic a bit better and to match the
unit/font tests in terms of folder structure for consistency.
Without these calls we'll not actually wait for saving to complete when document destruction runs; compare with other `WorkerTask`-usage in this file.
While I cannot imagine that this has caused any problems for library users, the code is however not technically correct as-is.
Note that PR 19212 tried to change the test-server to fix the intermittent failure in Google Chrome, however that unfortunately caused *other* unit-tests to start failing.
As long as this unit-test still runs successfully in Mozilla Firefox that should be enough, and it doesn't seem like a good use of our time to hunt down a bug that only happens in Google Chrome.
When computing the left offset of the highlighted text, we cannot use
.offsetLeft because the text might have been scaled through CSS, and it
needs to be taken into account.
Use `.getClientRects()`/`.getBoundingClientRect()` instead, which will
return measurements scaled appropriately.
This bug only seems to reproduce in Google Chrome, since browsers apparently sort response headers differently.
When the issue occurs the "raw" response headers string looks like this:
```
content-length: 525404\r\ncontent-type: \r\n
```
and since we trim *any* leading/trailing white-space characters the "content-type" header isn't detected correctly, which thus leads to `new Headers(...)` throwing.
Hence we'll keep regular spaces at the end of the "raw" response headers string, while still removing all other kinds of trailing white-space characters.
*Note:* The response headers parsing was based on https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/getAllResponseHeaders#examples
- it's now possible to start a drawing with a pen and use fingers to zoom
or scroll without interacting with the current drawing;
- it's now possible to draw with a finger and them zoom with two fingers.
When an editor is moved with the keyboard or in dragging it, it is moved in the DOM in order
to make screen readers happy. But this move is slightly postponed thanks to a setTimeout(..., 0).
The failures were very likely due to the fact that intermittently the DOM move was done in
the middle of the next key sequence which was making the move on screen failing.
Instead of conditionally checking if the `.cavnasWrapper` already
has a child element and then inserting the `canvas` before it, we can
use `.prepend` which always injects the new element as the first
child.
While drawing, in zooming fast enough, it's possible, intermittently, to have the canvas
after the svg which makes the svg invisible.
So this patch makes sure to have the canvas at the right position.
It was due the resize observer which is removed thanks to this patch.
In order to reuse the dragAndDrop function in test, this patch slighty refactors it
in order to make it easier to use.
The `Path2D` glyph-objects are cached on the `FontFaceObject`-instance, so we can save a little bit of memory by removing the raw path-strings once they're no longer needed.
The `must check input for US zip format` integration test fails pretty
consistently in Puppeteer 23.4.0+ with `Expected '12341' to equal
'12345'`. This is reproducible with the `pdf.sandbox.external.js` hack
from https://github.com/mozilla/pdf.js/issues/18396#issuecomment-2211273743.
Investigation uncovered two issues at play here:
1. We do two `clearInput` calls, but don't await processing of the two
sandbox events that are triggered by that action. The three tests that
use `issue14307.pdf` are in different `describe` blocks and therefore
reload the PDF file, so we can simply remove those calls because the
inputs are already empty by default.
2. We don't await processing of the sandbox events that occur after
switching to another text field. This causes the expectation failure
because the typing actions will happen too soon and interfere with
the sandbox event processing. We solve the issue by explicitly
awaiting the sandbox roundtrip.
Moreover, similar to PR #19001 and #18399 we remove any remaining room
for intermittent issues by directly checking for the expected value,
which also results in shorter code.
- Warn if the "regular" PDF.js build is used in Node.js environments, since that won't load any of the relevant polyfills.
- Warn if the `require` function cannot be accessed, since currently we're just "swallowing" any errors.
This requires two changes on our side:
- The order of exports in `web/viewer{-geckoview}.js` changes slightly
because `eslint-plugin-perfectionist` aligned the sorting order with
the `eslint-plugin-sort-exports` plugin we used before. This restores
the change from commit 347f155.
- The `eslint-plugin-import` plugin contains a bug that causes the new
version of `eslint-plugin-perfectionist` to be reported as unresolved.
This issue is tracked upstream, and since the plugin works fine we
can simply extend the ignore list we already have to avoid this error
until the upstream bug is fixed.
These DOM elements are `input type="checkbox"` and (natively) only support being toggled with the `Space` key, however we can extend an existing event-listener to "manually" support the `Enter` key as well.
Converting errors to string drops their stack trace, making it more
difficult to debug their actual reason. We can instead pass the error
objects as-is to console.warn/error, so that Firefox/Chrome devtools
will show both the stack trace of the console.warn/error call, and the
original stack trace of the error.
This commit also enables the `unicorn/no-console-spaces` ESLint rule,
which avoids accidental extra spaces when passing multiple parameters to
`console.*` methods.
This helps ensure that loading errors are always handled correctly, and note that both `PDFNetworkStreamFullRequestReader` and `PDFNetworkStreamRangeRequestReader` already provided such a callback.
Similar to the regular toolbarButtons that can be toggled, this ensure that it's always possible to tell when the findbar "buttons" are hovered/focused.
When a user deletes any number of annotations, they are notified of the action
by a popup message with an undo button. Besides that, this change reuses the
existing messageBar CSS class from the new alt-text dialog as much as possible.
Move .messageBar out of .dialog into its own standalone class in order
to reuse as much of it for the upcoming feature for an undo message for
annotations.
Some tests rely on the presence of a server that serves PDF files.
When tests are run from a web browser, the test files and PDF files are
served by the same server (WebServer), but in Node.js that server is not
around.
Currently, the tests that depend on it start a minimal Node.js server
that re-implements part of the functionality from WebServer.
To avoid code duplication when tests depend on more complex behaviors,
this patch replaces createTemporaryNodeServer with the existing
WebServer, wrapped in a new test utility that has the same interface in
Node.js and non-Node.js environments (=TestPdfsServer).
This patch has been tested by running the refactored tests in the
following three configurations:
1. From the browser:
- http://localhost:8888/test/unit/unit_test.html?spec=api
- http://localhost:8888/test/unit/unit_test.html?spec=fetch_stream
2. Run specific tests directly with jasmine without legacy bundling:
`JASMINE_CONFIG_PATH=test/unit/clitests.json ./node_modules/.bin/jasmine --filter='^api|^fetch_stream'`
3. `gulp unittestcli`
This allows us to remove an ESLint disable-statement for `arrow-body-style`, without affecting readability of the code, and fetching the metadata and the page in parallel should be a *tiny* bit more efficient as well.
- Use `this` in all scopes where that's possible, to avoid having to spell out `WorkerMessageHandler` everywhere.
- Inline the `isMessagePort` helper function, since there's only a single call-site.
- Use a static initialization block to move more code into the `WorkerMessageHandler` class itself.
This slightly shortens the code, in various `destroy`-methods, which cannot hurt.
Also, use pre-processor checks to simplify `PDFDocumentLoadingTask.destroy` in the Firefox PDF Viewer since the `PDFWorker.fromPort`-method isn't used there.
The date was create in UTC+0 and then amended in using set-Month/Date which take into account
the user timezone.
With this patch we build all the date in the user timezone.
It helps to slightly decrease memory use in reducing the number of created arrays.
In searching for "a" in pdf.pdf, the time spent in getOriginalIndex is decreased by
around 30%.
This patch makes a clear separation between the way to draw and the editing stuff.
It adds a class DrawEditor which should be extended in order to create new drawing tools.
As an example, the ink tool has been rewritten in order to use it.
Given that `browsertest` repeatedly timeout in Google Chrome, and considering that Firefox is the primary development target, we stop running them on the bots to avoid having to repeatedly deal with this.
Note that we already disabled these tests *on Windows* almost three years ago, because of stability issues; see PR 14392.
test/unit/api_spec.js is the only JS file in the tree with trailing
whitespace. Because `trim_trailing_whitespace = true` in .editorconfig,
any editor supporting EditorConfig would trim whitespace when the file
is changed, which results in test failures.
This commit fixes the issue by trimming the trailing whitespace and
adjusting the test expectations.
The `Array` type takes one parameter that describes the possible types
of the inner elements. This parameter may exist of pipes to indicate
multiple possible types. However, the current type definition provides
multiple parameters; this is incorrect syntax, and TypeScript 5.7+ now
fails on this due to stricter syntax validation.
This commit fixes the issue by changing the type definition to the
proper syntax, which together with the accompanying comment about the
contents of the fingerprints array should document it sufficiently.
The following cases are excluded in the patch:
- The Firefox PDF Viewer, since it has been fixed on the platform side already; please see https://bugzilla.mozilla.org/show_bug.cgi?id=1683940
- The `PDFNodeStream`-implementation, used in Node.js environments, since after recent changes that code only supports `file://`-URLs.
Also updates the `PDFNetworkStreamFullRequestReader.read`-method to await the headers before returning any data, similar to the implementation in `src/display/fetch_stream.js`.
*Note:* The relevant unit-tests are updated to await the `headersReady` Promise before dispatching range requests, since that's consistent with the actual usage in the `src/`-folder.
The test-only createTemporaryNodeServer helper featured a path traversal
vulnerability. This enables attackers with network access to the device
to read arbitrary files while unit tests are running that activate this
test server.
This patch fixes the issue by validation of paths.
To test this vulnerability before the patch:
1. Run the test-only server:
```
node -e 'console.log(require("./test/unit/test_utils.js").createTemporaryNodeServer().port)
```
2. From another terminal, send the following request (modify the port to
the port reported in the previous step):
```
curl --path-as-is http://localhost:45755/../../package.json
```
Before the patch, the second step would traverse the directory, and
return results from the root of the PDF.js repository, instead of files
within test/pdfs/.
With the patch, the server refuses the request with HTTP status 400.
This is fairly old code, and by making the function `async` we can handle initialization errors "automatically" without the need for try-catch statements.
and tweak a bit the highlight one (e.g. it's useless to have 64 bits floating point numbers
when 32 bits ones are enough).
It's a required step for the refactoring of the ink tool (in order to use the draw layer).
It avoids to call several functions acting on the same SVG element.
We can remove most feature testing from this helper function, with the exception of `randomUUID` since that's only available in "secure contexts", and also remove the fallback code-path.
Note that this code was only added for Node.js compatibility, and it's no longer necessary now that the minimum support version is `20`; see also https://developer.mozilla.org/en-US/docs/Web/API/Crypto#browser_compatibility
Finally, this patch also adds a basic unit-test for the helper function.
JSON imports are now supported by all tools used in PDF.js' build
process. The `chromecom.js` file is bundled by webpack and
import attributes are thus removed, so browser compatibility for this
new syntax is not relevant.
This integration test fails intermittently, locally at least in Chrome
with Puppeteer 23.4.0+, with the following errors:
```
In chrome: Expected '123Hello' to equal 'Hello123'.
In chrome: Expected '123Hello' to equal '123'.
```
This happens because the test before it left queued sandbox events
behind. We don't close the document between tests, so those get run
when we click the textbox in this test and that interferes with our
selection/typing actions. This commit fixes the issue by flushing the
queued sandbox events in the first test, which makes sure that state
no longer leaks through to the next test and thus improves isolation.
Morever, similar to commit 3adf8b6 we use safer assertions to avoid
further intermittent failures, and we replace the `page.$eval` call
with a simpler Home button push like we already do in e.g. the test
helpers. This combined makes the code shorter and simpler.
In PR #11300 Gitpod support got introduced, but we re-evaluated that
decision in #11732. In PR #11800 the support was partially reverted,
but the actual Gitpod files were kept to not outright break potential
workflows because at the time we were not sure if, and if so how often,
Gitpod was actually used for contributing to PDF.js.
However, in addition to the concerns mentioned in #11732 after five
years we haven't seen any contributions that clearly originated from
Gitpod, and the files have not been updated after e.g. PR #11807 and
PR #17913 because it's not a workflow that we maintain or are able
to test (nor have we seen Gitpod community contributions for this).
This commit therefore removes the remaining Gitpod files to reduce
maintainance burden for PDF.js. Note that users of Gitpod can still
contribute to PDF.js via the platform; we just don't provide/manage
workspace files from this repository anymore.
- Use optional chaining when clearing various data-structures, since that's slightly shorter.
- Ensure that checking for the existence of `#_hcmCache`-data won't cause it to be initialized. (Note how `this.#hcmCache` is actually a getter.)
- Actually reset the `#_hcmCache`-data when that's appropriate, e.g. when closing the PDF document.
- Ensure that `pdfjsTestingUtils` is available when running benchmarking, since that shouldn't be done in TESTING-mode.
- Exclude the `test/stats/results/` folder from linting, since it'll contain *generated* JSON-files.
When iterating through `useCMap` the value is already available, without having to manually invoke the `lookup`-method.
While this will likely not affect performance in any noticeable way, it's nonetheless unnecessary to lookup an already available value twice.
This was previously attempted in PR 13371, but had to be reverted because of issues related to SystemJS (which has since been removed).
Also, while unrelated, shortens an existing conditional assignment.
The purpose of these changes is to make it more difficult to accidentally include logging statements, used during development and debugging, when submitting patches for review.
For (almost) all code residing in the `src/` folder we should use our existing helper functions to ensure that all logging can be controlled via the `verbosity` API-option.
For the `test/unit/` respectively `test/integration/` folders we shouldn't need any "normal" logging, but it should be OK to print the *occasional* warning/error message.
Please find additional details about the ESLint rule at https://eslint.org/docs/latest/rules/no-console
Given that there are multiple issues with `ImageDecoder` in Chromium browsers, affecting both BMP and JPEG images, for now we (by default) disable that functionality there to avoid problems.
This also means that we can remove the previously added, and separate, `isChrome` API-option.
Python 3.13 is the current version and was released over a month ago
(see https://devguide.python.org/versions). The dependencies we use now
support Python 3.13, most importantly `fonttools` which uses OS-specific
builds and for which compatibility got introduced in
https://github.com/fonttools/fonttools/pull/3656 and the corresponding
`cp313` wheels for all distributions are published on
https://pypi.org/project/fonttools/#files.
Moreover, we fix forgotten `npx` usage in the font tests README which
was encountered while testing this patch.
This is unblocked now that all dependencies have been updated and the
flat configuration format (compatible with ESLint 8 and 9) was
introduced first. The following deprecation warnings during `npm
install` are resolved by this upgrade:
```
npm warn deprecated @humanwhocodes/config-array@0.13.0: Use @eslint/config-array instead
npm warn deprecated @humanwhocodes/object-schema@2.0.3: Use @eslint/object-schema instead
npm warn deprecated eslint@8.57.1: This version is no longer supported. Please see https://eslint.org/version-support for other options.
```
Note that according to https://eslint.org/version-support ESLint 8 is
officially EOL now, and ESLint 9 has been released for over seven
months and is the only officially supported version.
Fixes#17928.
This allows end-users to forcibly disable `ImageDecoder` usage, even if the browser appears to support it (similar to the pre-existing option for `OffscreenCanvas`).
Flat config is the new config system used by ESLint 9.
To make the migration easier, they also added
flat config support to ESLint 8.
This commit migrates the various ESLint configs in the repository to use
the new system, **without** upgrading to ESLint 9 yet.
It fixes#19008.
In Firefox on mac, the default padding is set to 4px and with Firefox for iOS, it's set to 13px.
The padding is useless for such buttons.
Given that `ImageData` has been supported for many years in all browsers, see [MDN](https://developer.mozilla.org/en-US/docs/Web/API/ImageData#browser_compatibility), we have a `typeof` check that's only necessary in Node.js environments.
Since the `@napi-rs/canvas` package provides that functionality, we can thus add an `ImageData` polyfill which allows us to ever so slightly simplify the code.
The `@napi-rs/canvas` package has fewer dependencies, which should *hopefully* make installing and using it easier for `pdfjs-dist` end-users. (Over the years we've seen, repeatedly, that `canvas` can be difficult to install successfully.)
Furthermore, this package includes more functionality (such as `Path2D`) which reduces the overall number of dependencies in the PDF.js project.
One point to note is that `@napi-rs/canvas` is a fair bit newer than `canvas`, and has a lot fewer users, however looking at the commit history it does seem to be actively maintained.
Note that I've successfully tested the [Node.js examples](https://github.com/mozilla/pdf.js/tree/master/examples/node), in particular the `pdf2png` one, with this patch applied and things appear to work fine.
Please see:
- https://www.npmjs.com/package/@napi-rs/canvas
- https://github.com/Brooooooklyn/canvas
Fixes https://github.com/mozilla/pdf.js/issues/18957https://github.com/mozilla/pdf.js/pull/18682 introduced a regression that causes the following error:
```
Uncaught TypeError: Failed to construct 'Headers': Invalid name
at PDFNetworkStreamFullRequestReader._onHeadersReceived (pdf.mjs:10214:29)
at NetworkManager.onStateChange (pdf.mjs:10103:22)
```
The mentioned PR replaced a call to `getResponseHeader()` with `getAllResponseHeaders()` without handling cases where it may return null or an empty string. Quote from the [docs](https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/getAllResponseHeaders#return_value):
> Returns:
>
>A string representing all of the response's headers (except those whose field name is Set-Cookie) separated by CRLF, or null if no response has been received. If a network error happened, an empty string is returned.
Run the following code and observe the error in the console. Note that the URL is intentionally set to an invalid value to simulate network error
```js
<script src="//mozilla.github.io/pdf.js/build/pdf.mjs" type="module"></script>
<script type="module">
var url = 'blob:';
pdfjsLib.GlobalWorkerOptions.workerSrc = '//mozilla.github.io/pdf.js/build/pdf.worker.mjs';
var loadingTask = pdfjsLib.getDocument(url);
loadingTask.promise
.then((pdf) => console.log('PDF loaded'))
.catch((reason) => console.error(reason));
</script>
```
The problem with the referenced PDF document has nothing to do with invalid dates, as the issue seems to suggest, but rather with the fact that it has neither an XRef table nor a trailer dictionary.
Given that crucial parts of the internal document structure is missing, you might argue that it's not really a PDF document.
In an attempt to support this kind of corruption, we'll simply iterate through all (previously found) XRef entries and pick one that *might* be a valid /Root dictionary.
There's obviously no guarantee that this works, and it might not be fast in larger PDF documents, but at least it cannot be any worse than *immediately* throwing `InvalidPDFException` as we previously did here.
*Please note:* I'm totally fine with this patch being rejected, since it's somewhat questionable if we should actually attempt to support "PDF documents" with this level of corruption.
This patch updates the minimum supported environments as follows:
- Node.js 20, which was released on 2023-04-18 and has now entered the "Maintenance"-phase; see https://github.com/nodejs/release#release-schedule
Furthermore, note also that Node.js 18 will fairly soon reach EOL.
This integration test fails intermittently because we're not
(correctly) awaiting the sandbox actions. The `27R` field in
`issue14862.pdf` triggers sandbox events for every typing action, but
for the backspace and "a" character typing actions we weren't awaiting
the sandbox trip at all, and for other places we weren't awaiting it
fully (causing some characters to be missed in the assertion).
This commit fixes the issues by using the appropriate helper functions,
similar to what we did in PR #18399. Not only is this shorter in terms
of code, but it also fixed the near-permafail for this test with newer
versions of Puppeteer.
- This helper function has only a single call-site, and the function is fairly short.
- It'll only be invoked if range requests are *disabled*, or if the entire PDF manages to load *before* the headers are resolved (which is very unlikely).
Hence, by default, this helper function is not invoked.
- By inlining the code we're able to utilize the existing error-handling at the call-site, rather than having to duplicate it, which further reduces the size of this code.
Finally, while slightly unrelated, this patch also adds optional chaining in one spot in the file (PR 16424 follow-up).
I discovered that doing skip-cache re-reloading of https://opensource.adobe.com/dc-acrobat-sdk-docs/pdfstandards/PDF32000_2008.pdf would *intermittently* cause (some of) the AnnotationLayers to break with errors printed in the console (see below).
In hindsight this bug is really obvious, however it took me quite some time to find it, since the `StructTreePage.prototype.serializable` getter will lookup various data and all of those cases can fail during loading when streaming and/or range requests are being used.
Finally, to prevent any future errors, ensure that the viewer won't break in these sort of situations.
```
Uncaught (in promise)
Object { message: "Missing data [19098296, 19098297)", name: "UnknownErrorException", details: "MissingDataException: Missing data [19098296, 19098297)", stack: "BaseExceptionClosure@resource://pdf.js/build/pdf.mjs:453:29\n@resource://pdf.js/build/pdf.mjs:456:2\n" }
viewer.mjs:8801:55
\#renderAnnotationLayer: "UnknownErrorException: Missing data [17552729, 17552730)". viewer.mjs:8737:15
Uncaught (in promise)
Object { message: "Missing data [17552729, 17552730)", name: "UnknownErrorException", details: "MissingDataException: Missing data [17552729, 17552730)", stack: "BaseExceptionClosure@resource://pdf.js/build/pdf.mjs:453:29\n@resource://pdf.js/build/pdf.mjs:456:2\n" }
viewer.mjs:8801:55
```
- Over time the number and size of these factories have increased, especially the `DOMFilterFactory` class, and this split should thus aid readability/maintainability of the code.
- By introducing a couple of new import maps we can avoid bundling the `DOMCMapReaderFactory`/`DOMStandardFontDataFactory` classes in the Firefox PDF Viewer, since they are dead code there given that worker-thread fetching is always being used.
- This patch has been successfully tested, by running `$ ./mach test toolkit/components/pdfjs/`, in a local Firefox artifact-build.
*Note:* This patch reduces the size of the `gulp mozcentral` output by `1.3` kilo-bytes, which isn't a lot but still cannot hurt.
This way we can directly throw Errors, rather than having to "manually" return rejected Promises, which is ever so slightly shorter.
Also, since `useWorkerFetch` is always true in MOZCENTRAL builds these message-handlers should not be invoked there.
One goal is to make the code for drawing with the Ink tool similar to the one to free highlighting:
it doesn't really make sense to have so different ways to do almost the same thing.
When the zoom level is high, it'll avoid to create a too big canvas covering all the page which consume
more memory, makes the drawing very slow and the overall user xp pretty bad.
A second goal is to be able to easily implement more drawing tools where we would just have to implement
how to draw from the pointer coordinates.
We can convert the handler to an `async` function, which removes the need to create a temporary Promise here.
Given the age of this code it shouldn't hurt to simplify it a little bit.
This allows using the new methods in browsers that support them, e.g. Firefox 133+, while still providing fallbacks where necessary; see https://github.com/tc39/proposal-arraybuffer-base64
*Please note:* These are not actual polyfills, but only implements what we need in the PDF.js code-base. Eventually this patch should be reverted, once support is generally available.
- Add explicit `length` validation of the /ID entries. Given the `EMPTY_FINGERPRINT` constant we're already *implicitly* assuming a particular length.
- Move the constants into the `fingerprints`-getter, since they're not used anywhere else.
- Replace the `hexString` helper function with the standard `Uint8Array.prototype.toHex` method; see https://github.com/tc39/proposal-arraybuffer-base64
This extends PR 13796 to also handle the case where sub-streams contain invalid data, i.e. anything that isn't a Stream, however please note that in these cases there's no guarantee that we'll render the page "correctly".
Note that Adobe Reader, i.e. the PDF reference implementation, cannot render the last page of the referenced PDF document.
Currently we manually localize and update the DOM-elements of the AltText-button, and it seems nicer to utilize Fluent "properly" for that task.
This can be achieved by introducing an explicit `span`-element on the AltText-button (similar to e.g. the regular toolbar-buttons), and adding a few more l10n-strings, since that allows just setting the `data-l10n-id`-attribute on all the relevant DOM-elements.
Finally, note how we no longer need to localize any strings eagerly when initializing the various editors.
Move the `ImageResizer._goodSquareLength` definition into the class itself, since the current position shouldn't be necessary, and also convert it into an actually private field.
It fixes#18956.
In the patch #18029, for performance reasons and because I thought it was useless, I deliberately chose to not fill the mask
with the backdrop color when it's full black: it was a bad idea.
So in this patch we always add the backdrop color to the mask.
After the binary CMap format had been added there were also some ideas about *maybe* providing other formats, see [here](https://github.com/mozilla/pdf.js/pull/8064#issuecomment-279730182), however that was over seven years ago and we still only use binary CMaps.
Hence it now seems reasonable to simplify the relevant code by removing `CMapCompressionType` and instead just use a boolean to indicate the type of the built-in CMaps.
Given that we've not shipped, nor used, anything except binary CMaps for years let's just cache them unconditionally (since that's a tiny bit less code).
The PDF document is clearly corrupt, since it has /FontFile2 entries that are Dictionaries which obviously isn't correct.
While there's obviously no guarantee that things will look perfect this way, actually rendering the text at all should be an improvement in general.
Unfortunately it turns out to be somewhat common for users to provide a bunch of "unrelated" information in this field, or even stating their entire problem there, rather than placing it under the appropriate headings further down in the template.
This moves more functionality into the base-class, rather than having to duplicate that in the extending classes.
For consistency, also updates the `BaseStandardFontDataFactory` and introduces more `async`/`await` in various relevant code.
With the recent re-factoring of the viewer CSS rules we now have some duplication of the `mask-image` definitions for the print/download buttons in the secondaryToolbar; note 17419de157/web/viewer.css (L1204-L1210)
The `getSpanRectFromText` helper function returns the location as float
values. This could be desirable in cases where the exact values matter
(for example during comparisons), but in the text layer tests we don't
need this precision. Moreover, the Puppeteer `page.mouse.move` API
apparently doesn't work correctly if float values are given as input.
Note that this test only failed because it couldn't move to the initial
selection position; any subsequent moves actually worked because the
`moveInSteps` helper function already rounded all values correctly.
This commit fixes the issue by consistently rounding all values that we
pass to Puppeteer's `page.mouse.move` API.
By using "data-l10n-attrs" it's possible to instruct Fluent to localize *custom* attributes, which means that we don't need to manually translate/update the "default-content" in FreeText editors.
The plugins have originally been introduced in commit d63da81 for the
`eslint-plugin-mozilla` dependency, but the `eslint-plugin-mozilla`
plugin got removed in commit be93d53 and we also don't use the plugins
ourselves in e.g. our `.eslintrc` files (as evidenced by `npx gulp lint`
not failing while it does fail if we remove any of the other plugins).
This way we allow the rest of the packages to be loaded successfully, such that e.g. the Node.js unit-tests work correctly.
Note that this occurred after updating the `node-canvas` package to version `3.0.0-rc2`, however it's not immediately clear to me if it's a problem there or in the `path2d` package; see also nilzona/path2d-polyfill/issues/84.
- Change the "message" event handler function to a private method.
- Remove the "message" event listener with an `AbortSignal`.
- Extend the `LoopbackPort`-class with `AbortSignal` support.
The code parses the /RBGroups entry in the OC configuration dict and adds the property `rbGroups' to instances of the OptionalContentGroup class. rbGroups takes an array of Sets, where each Set instance represents an RB group the OptionalContentGroup instance is a member of. Such a Set instance contains all OCG ids within the corresponding RB group. RB groups an OCG is associated with are processed when its visibility is set to true, as required by the PDF spec.
Given that the sub-title of that document is "Public domain texts for young people." and that the images have clear sources at the end of the document, it should (hopefully) be OK to add it to the repository rather than relying on a linked test-case.
Since this value is used to allocate an array, it makes sense to avoid to use too much memory.
From the specs, this value must be in the range [0; 255] (see section 8.6.6.3).
This patch removes the unused property 'highVal'.
When playing with a pen, I noticed that sometimes a free highlight has still its
gray outline when an other one is drawn: for any reason the pointerleave event isn't
triggered.
This is similar to a lot of other code, where we've been replacing explicit `removeEventListener`-calls.
Given the somewhat limited availability of `AbortSignal.any()`, see [MDN](https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/any_static#browser_compatibility), this event listener will no longer be immediately removed in older browsers (however that should be fine).
After PR 18845 we're accessing this getter more, hence it seems like a good idea to ensure that the initial `formInfo` access is covered as well.
While unlikely to be a problem in practice, at least theoretically that data may not be available and the code in `fieldObjects` could thus currently be *unintentionally* invoked more than once.
The default `page.type()` API from Puppeteer works for text fields that
only dispatch a sandbox event on e.g. focus loss (i.e. after all
characters have been inserted), and for those we can also use the
default typing delay from Puppeteer instead of defining our own value.
However, it doesn't work correctly for text fields where every character
insertion dispatches a sandbox event. This is because processing the
sandbox event takes some time and Puppeteer must wait for that before it
can (safely) insert the next character. This commit therefore introduces
a helper function to type a given value correctly in such text fields.
Not only does this fix intermittent failures if our delay was too low
for sandbox processing to complete, but it also speeds up the tests by
eliminating our delays in places where they were (much) higher than
necessary. In total the runtime of the scripting integration test suite
goes from 137 seconds before this patch to 100 seconds after this patch.
Note that the new version of `eslint-plugin-import` contains support
for ESLint 9.
Moreover, the new version of Babel removed the leading space for import
comments (see https://github.com/babel/babel/pull/16780), so we update
the corresponding test expectation to match this.
Fixes a part of #17928.
A recent change in Firefox induced too much difference between the text widths computed in using a Canvas
and the ones computed by the text layout engine when rendering the text layer. Consequently, the
text selection can be bad on Windows with some fonts like Arial or Consolas.
This patch is a workaround to try to use in first place some fonts which don't have the problem.
Strangely enough the code works just fine as-is in the GENERIC viewer, so there must be some difference between the Firefox built-in Fluent implementation and the Fluent.js one.
It seems that we can work-around the problem by handling this l10n-arg the same way that we handle dates in the AnnotationLayer, and testing this with the Firefox Devtools it seems that it should work.
It fixes#18849.
When such an annotation is deleted, we make sure that there are some data
to restore.
The version of this patch was making undoing a svg deletion buggy, so it's fixed now and
an integration test has been added for this case.
With the changes made in PR 18385 the `toolbarViewer` element is now shorter than before, since the padding is applied on its `toolbarContainer` parent-element instead.
This causes two separate regressions:
- Clicking at the very top/bottom of the toolbar no longer closes the secondaryToolbar like previously.
- The `CaretBrowsingMode`-constructor no longer computes the toolbar-height correctly.
Given how/where the `container`-property is being used these changes *should* thus be safe.
The current implementation won't work correctly in some cases, e.g. if RBGroups are present, which means that it's possible for the UI to get out-of-sync with the actual optionalContent-state.
To avoid querying the DOM unnecessarily we cache the last known UI-state and compare with the actual optionalContent-state, which thus replaces the previously used hash-comparison.
This patch updates the minimum supported browsers as follows:
- Google Chrome 103[1], which was released on 2022-06-21; see https://chromereleases.googleblog.com/2022/06/stable-channel-update-for-desktop_21.html
Note that nowadays we usually try, where feasible and possible, to support browsers that are about two years old. By limiting support to only "recent" browsers we reduce the risk of holding back improvements of the *built-in* Firefox PDF Viewer, and also (significantly) reduce the maintenance/support burden for the PDF.js contributors.
*Please note:* As always, the minimum supported browser version assumes that a `legacy`-build of the PDF.js library is being used; see https://github.com/mozilla/pdf.js/wiki/Frequently-Asked-Questions#faq-support
---
[1] This is consistent with the minimum supported version in the recently updated Google Chrome addon.
We disable any non-default CursorTool in PresentationMode and during Editing, since they don't make sense there and to prevent problems such as e.g. [bug 1792422](https://bugzilla.mozilla.org/show_bug.cgi?id=1792422).
Hence it seems like a good idea to *also* disable the relevant SecondaryToolbar-buttons, to avoid the user being surprised that the CursorTools-buttons do nothing if clicked.
Given the following excerpt from the [Wikipedia article](https://en.wikipedia.org/wiki/Gill_Sans), mapping this to Helvetica should hopefully be fine:
> It has been described as "the British Helvetica" because of its lasting popularity in British design.
Given that the `WorkerTransport`-constructor will access all possible factory-instances, let's ensure that the `transportFactory`-object always has a consistent shape regardless of other options.
Also, since `useWorkerFetch` is always true in MOZCENTRAL builds we never need to create `CMapReaderFactory`/`StandardFontDataFactory`-instances there.
The first goal of this patch was to remove the tabindex because it helps
to improve overall a11y. That led to move some html elements associated
with the buttons which helped to position these elements relatively to their
buttons.
Consequently it was easy to change the toolbar height (configurable in Firefox
with the pref browser.uidensity): it's the second goal of this patch.
For a11y reasons we want to be able to change the height of the toolbar to make
the buttons larger.
This is unblocked because in commit bb302dd the default value for the
constructor got removed, which apparently confused TypeScript before.
Fixes#18770.
This unifies the various factory-options, since it's consistent with `CMapReaderFactory`/`StandardFontDataFactory`, and ensures that any needed parameters will always be consistently provided when creating `CanvasFactory`/`FilterFactory`-instances.
As shown in the modified example this may simplify some custom implementations, since we now provide the ability to access the `CanvasFactory`-instance used with a particular `getDocument`-invocation.
Currently, the css for a separator is something like { height: 1px; background-color: ... }.
But its rendering depends on its position on the screen.
So instead of setting the height to 1px, we just set something like { border-top: 1px solid ...; },
this way the final rendering is exactly the same for all the separators.
This code is quite old and has been moved/re-factored a few times over the years, however we can simplify this even further since we don't actually need a function to determine what NetworkStream-implementation to use.
After the removal of 'unsafe-eval' CSP in #18651, WebAssembly fails to
load, resulting in issues such as seen in #18457.
Manifest Version 3 does not allow 'unsafe-eval', does accept the more
specific 'wasm-unsafe-eval' as of Chrome 103. Note that manifest.json
already sets minimum_chrome_version to 103.
This patch also adds `object-src 'self'` because it was required until
Chrome 110. As of Chrome 111, the default is `object-src 'self'` and
`object-src` is no longer required. We could drop `object-src` in the
future, but for now we need to include it to support Chrome 103 - 110.
Note that the textContent is returned in "chunks" from the API, through the use of `ReadableStream`s, and on the main-thread we're (normally) using just one temporary canvas in order to measure the size of the textLayer `span`s; see the [`#layout`](5b4c2fe1a8/src/display/text_layer.js (L396-L428)) method.
*Order of events, for parallel textLayer rendering:*
1. Call [`render`](5b4c2fe1a8/src/display/text_layer.js (L155-L177)) of the textLayer for page A.
2. Immediately call `render` of the textLayer for page B.
3. The first text-chunk for pageA arrives, and it's parsed/layout which means updating the cached [fontSize/fontFamily](5b4c2fe1a8/src/display/text_layer.js (L409-L413)) for the textLayer of page A.
4. The first text-chunk for pageB arrives, which means updating the cached fontSize/fontFamily *for the textLayer of page B* since this data is unique to each `TextLayer`-instance.
5. The second text-chunk for pageA arrives, and we don't update the canvas-font since the cached fontSize/fontFamily still apply from step 3 above.
Where this potentially breaks down is between the last steps, since we're using just one temporary canvas for all measurements but have *individual* fontSize/fontFamily caches for each textLayer.
Hence it's possible that the canvas-font has actually changed, despite the cached values suggesting otherwise, and to address this we instead cache the fontSize/fontFamily globally through a new (static) helper method.
*Note:* Includes a basic unit-test, using dummy text-content, which fails on `master` and passes with this patch.
Finally, pun intended, ensure that temporary textLayer-data is cleared *before* the `render`-promise resolves to avoid any intermittent problems in the unit-tests.
Fix regression from #18711. `urlFilter` is a key of `condition`, not of
the `rule`. Consequently, the feature detection method failed to detect
the availability of the feature in Chrome 128+.
The minimum required version is Chrome 103 because wildcard support for
web_accessible_resources[].extension_id was introduced in 103, in
c9caeb1a08
A way to broaden compatibility is to drop that key. By doing so, the
minimum required Chrome version is then 96, because of the
chrome.storage.session API (and declarativeNetRequestWithHostAccess).
Since gulpfile.js already defines "Chrome >= 98" and there is no real
point in expanding support to older versions, I'm just setting the
minimum version to 103.
- Replace DOM-based pdfHandler.html (background page) with background.js
(extension service worker).
- Adjust logic of background scripts to account for the fact that the
scripts can execute repeatedly during a browser session. Primarily,
register relevant extension event handlers at the top level and use
in-memory storage.session API to keep track of initialization state.
- Extension URL router: replace blocking webRequest with the service
worker-specific "fetch" event.
- PDF detection: replace blocking webRequest with declarativeNetRequest.
This requires Chrome 128+. The next commit will add a fallback for
earlier Chrome versions.
In MV3, the background script is a service worker. localStorage is not
supported in service workers. Switch to storage.local instead.
Data migration is not implemented because it is not needed due to the
privacy-friendly design of the telemetry: In practice the data is reset
about every 4 weeks, when the major version of Chrome is updated.
restoretab.js was added in https://github.com/mozilla/pdf.js/pull/6233
with the purpose of restoring lost tabs when Chrome closes all extension
tabs when it reloads the extension. This forced reload can happen when
the user toggles the "Allow access to file URLs" option.
This logic does not work any more, and since the use of localStorage is
a blocker in migrating to MV3, this patch just drops the logic.
MV3 does not support chrome_style in options_ui. Remove it and replace
it with the minimal amount of styles that still has some spacing around
the individual settings for readability.
webRequestBlocking is unavailable in MV3. Non-blocking webRequest can
still be used to detect the Referer, but we have to use
declarativeNetRequest to change the Referer header as needed.
*Please note:* As a general rule we probably don't need to fix things affecting *custom* implementations of the default viewer, but in this case a "targeted" fix seem possible that shouldn't (famous last words) break anything else.
Ensure that the `.visibleMediumView` class, which is used when the viewer becomes narrow, cannot override the `display`-value for DOM elements that are being *explicitly* hidden either through the associated attribute or class.
The idea is to insert a span in the text layer with an aria-role set to img
and use the bounding box provided by the attribute field in the tag dict in
order to have non-null dimensions for the image to make it "visible".
In hindsight it occurred to me that there's a couple of smaller issues with this method after it's made asynchronous (in PR 18658).
- If the `render`-method is invoked back-to-back the existing caching doesn't guarantee that re-parsing won't occur, which we can address by introducing a new (private) promise.
- If there's any errors fetching and/or parsing the structTree-data, we'd attempt to parse it again on re-rendering despite that being pointless.
It has been tested with Voice Over (mac) and with NVDA (windows).
When an added stamp annotation is focused, the screen reader will announce
that it's a figure containing a graphic with the added alt-text.
The `Headers` functionality is now available in all browsers/environments that we support, which allows us to consolidate and simplify how the `httpHeaders` API-option is handled; see https://developer.mozilla.org/en-US/docs/Web/API/Headers#browser_compatibility
Also, simplifies the old `NetworkManager`-constructor a little bit.
It was recently brought to my attention that using partial or generated localization ids is bad for maintainability, hence this patch goes through the code-base and replaces any such occurrences.
Currently we repeat virtually the same http/https request code in two different classes in this file, which seems unnecessary and it leads to more code.
With the introduction of Fluent the `getLanguage`-method became synchronous, hence it no longer seems necessary to do the metric-locale check eagerly in the constructor and it can instead be "delayed" until actually needed.
It was recently brought to my attention that using partial or generated localization ids is bad for maintainability, which means that PR 18636 wasn't the correct thing to do.
However, just reverting that one doesn't really fix the problems which is why this patch updates *every* l10n-id in the `PDFDocumentProperties` class (but doesn't touch any `viewer.ftl`-files). Obviously this leads to more verbose code, but that cannot really be helped.
We can remove the `reset`-parameter, since it's redundant, given that it's only used after `PDFDocumentProperties.#reset` has been invoked which means that `this.#fieldData === null` which is equivalent to resetting.
Also, we don't need to have two separate loops in order to update the UI in this method.
Finally, inline the `DEFAULT_FIELD_CONTENT` constant now that it's only used once.
The Node.js url.parse API (https://nodejs.org/api/url.html#urlparseurlstring-parsequerystring-slashesdenotehost)
is deprecated because it's prone to security issues (to the point that Node.js doesn't even publish CVEs for it anymore).
The official reccomendation is to instead use the global URL constructor, available both in Node.js and in browsers.
Node.js filesystem APIs accept URL objects as parameter, so this also avoids a few URL->filepath conversions.
This l10n-string is being re-defined once for every editor, i.e. currently four times, which seems unnecessary.
To avoid having to check if this l10n-string exists first, we can utilize rest parameters to move it into the `AnnotationEditor._l10nPromise` Map-definition instead.
Currently we manually localize and update the DOM-elements of the editor-resizers, and it seems nicer to utilize Fluent for that task.
This can be achieved by updating the l10n-strings to directly target the `aria-label` and then just setting the `data-l10n-id` on the DOM-elements.
In preparation for migrating the Chrome extension to Manifest Version 3,
this patch removes parts of the manifest that are obsolete and/or
unsupported in MV3.
Remove ftp mentions: ftp was dropped from 6 years ago, in Chrome 59.
Remove file_browser_handlers: does not work on Chrome OS according to
https://github.com/mozilla/pdf.js/issues/14161 . MV3 replacement needs
a different manifest key and logic, which will be added later.
Remove content_security_policy: MV3 does not support unsafe-eval CSP,
and PDF.js does not require it. This may result in a mild performance
degradation in PDFs that contain PostScript.
Remove page_action logic: When this logic was originally introduced,
Chrome showed page action buttons in the address bar, which enabled
the extension to display the button on specific PDF viewer tabs only.
In Chrome 49 (2016), pageActions were dropped from the address bar and
put in an UI area that is hidden by default. The user can pin the button
to be unconditionally visible on the toolbar, which is distracting.
Because the UX is no longer serving the original needs, this patch
removes page_action from the manifest.
This major version contains three breaking changes that impact us:
- The `product` option has been renamed to the more suitable `browser`.
- The `page.screenshot()` API returns a `Uint8Array` instead of a
`Buffer`, but since `pngjs` requires a `Buffer` object we need to do
the conversion using `Buffer.from()` before passing data to `pngjs`.
- The browser configuration should be set using a configuration file
instead of environment variables. Note that as a bonus this allows us
to remove the `cross-env` dependency since that was only used to set
the Puppeteer environment variable equally for all operating systems.
For more information about the changes between the old and new Puppeteer
versions refer to https://github.com/puppeteer/puppeteer/releases.
The `AnnotationLayer` may not display correctly formatted data in PopupAnnotations, especially in the GENERIC viewer, since it's using native methods[1] that depend on the *browser* locale instead of the viewer locale as intended.
With Fluent we're able to improve things since it's got built-in support for formatting dates. Not only does this simplify the JavaScript code slightly, but it also gives the localizer more fine-grained control of the desired output.
Please find additional information here:
- https://projectfluent.org/fluent/guide/builtins.html
- https://projectfluent.org/fluent/guide/functions.html
---
[1] `toLocaleDateString`, and `toLocaleTimeString`.
The `PDFDocumentProperties` dialog may not display correctly formatted data, especially in the GENERIC viewer, since it's using native methods[1] that depend on the *browser* locale instead of the viewer locale as intended.
At the time when this dialog was introduced that was probably all we could easily do, but with Fluent we're able to improve things since it's got built-in support for formatting numbers and dates. Not only does this simplify the JavaScript code, but it also gives the localizer more fine-grained control of the desired output.
Please find additional information here:
- https://projectfluent.org/fluent/guide/builtins.html
- https://projectfluent.org/fluent/guide/functions.html
---
[1] `toLocaleString`, `toLocaleDateString`, and `toLocaleTimeString`.
Currently we *manually* fetch the "pdfjs-additional-layers" string and update the DOM-element, which was needed since we want to avoid triggering a bunch of otherwise unnecessary translation when appending the entire layer-tree to the DOM.
By introducing a new helper method in the `L10n`-class we can avoid this, and instead use a "data-l10n-id" attribute on the element (as most other viewer code does nowadays).
Given the length of the l10n-strings we can slightly reduce verbosity, and thus overall code-size, by introducing a helper method for fetching l10n-data.
While testing this I stumbled upon an issue in the `L10n`-class, where an optional chaining operator was placed incorrectly since the underlying method always return an Array; see 48e2a62ed4/fluent-dom/src/localization.js (L38-L77)
- When adding page dict candidates to the lookup tree, also initiate fetching them from xref, so if they are not yet loaded at all, the XHR will be sent
- Only at the top level - assume that if there is a /Pages tree, it is sensibly structured and the number of requests won't be too bad
- We can then await on the cached Promise without making the requests pipeline
- This has a significant performance improvement for load-on-demand (i.e. with auto-fetch turned off) when a PDF has a large number of pages in the top level /Pages collection, and those pages are spread through a file, so every candidate needs to be fetched separately
- PDFs with many pages where each page is a big image and all the pages are at the top level are quite a common output for digitisation programmes
- I would have liked to do something like "if it's the top level collection and page count = number of kids, then just fetch that page without traversing the tree" but unfortunately I agree with comments on #8088 that there is no good general solution to allow for /Pages nodes with empty /Kids arrays
Given that we handle non-embedded Calibri fonts as "mapped to standard font", we really ought to be able to use the same glyph mapping as for an actual standard font.
Note that this actually improves consistency in the code, given how we already handle such fonts if they happen to be of the `CIDFontType2` type; see b47c7eca83/src/core/fonts.js (L1186-L1190)
This integration test fails intermittently because we cache the initial
total value to be able to compare it to the new total value at the end
of the test to check that it's different before doing the assertions.
However, this doesn't work as expected because the second `clearInput`
call triggers an intermediate total value calculation because it clicks
on another input field and that triggers a sandbox event.
This results in the `waitForFunction` calls always resolving immediately
and since we don't use other means of waiting until the calculation is
done (using e.g. `waitForSandboxTrip`) we basically rely on the time
between the final click and the assertions to be enough for the sandbox
to do its work. If it's is not done in that time, we do the assertions
with older values and that makes the test fail.
This commit fixes the issue by simply waiting for the total value to be
what we expect it to be. This requires less code, is more consistent
with the other integration tests and removes the possibility of doing
assertions against older values.
In PR #18574 setting `window.uiManager` was moved into the `src` folder
to avoid intermittent integration test failures because at the time we
lacked a way to register event listeners early (before PDF.js loads).
However, in PR #18617 this functionality got introduced, so we can now
use the new way of setting up the event bus in the tests to move this
back to the `test` folder again and to reduce the amount of test-only
code in the main codebase as discussed in PR #18574.
Partially reverts e037c5711d3d2413669e9b6c275986adf24a295b.
The function evaluateOnNewDocument in Puppeteer allow us to execute some js before the pdf.js one
is loaded.
It allows us to stub some setters before there are used and then set some event handlers very soon.
We introduced quite a few new strings recently, but during the last few
rounds of updates we didn't see new translations updates becoming
available. I only just now recalled the announcement that Mozilla is
moving from Mercurial to Git, and indeed the Mercurial page at
https://hg.mozilla.org/l10n-central hasn't been updated since July
anymore and that's were we used to pull our translations from.
This commit fixes the issue by changing the URLs to the Mozilla Git
repositories hosted on GitHub instead.
The way that the debugging hash-parameter parsing is implemented leads to a lot of boilerplate code in this method, since *most* of the cases are very similar.[1]
With just a few exceptions most of the options can be handled automatically, by defining an appropriate checker for each option.
---
[1] With the recent introduction of TESTING-only options the size of this method increased a fair bit.
The problem seems to be caused by the browser trying to "restore" editing input-elements, in the various toolbars, to their previous values when the tab is re-opened.
Hence the simplest solution appears to be to move the event handling into the editor-code, which is also less code overall, since the listener thus won't be registered early enough for the problem to appear.
This patch allows embedders of PDF.js to provide custom match
logic for seaching in PDFs. This is done by subclassing the
PDFFindController class and overriding the `match` method.
`match` is called once per PDF page, receives as parameters the
search query, the page contents, and the page index, and returns
an array of { index, length } objects representing the search
results.
This commit fixes a regression from #18596 where the "Add image" button
styles broke because it's a labeled toolbar button but the labeled
toolbar button CSS rules were only scoped to the secondary toolbar.
However, nowadays labeled toolbar buttons are also used in e.g. the
editor parameters toolbar, and this highlighted that we didn't clearly
encode the concept of labeled toolbar buttons in the CSS so far.
This patch extracts the CSS rules for labeled toolbar buttons that were
previously limited to the secondary toolbar into a dedicated generic
class that can be applied on top of any unlabeled toolbar button to
convert it to a labeled toolbar button, regardless of its position in
the DOM. This also makes the distinction clearer in the HTML, and the
new CSS scope for the toolbar buttons lays the foundation for combining
the other toolbar buttons rules in there later on.
Co-authored-by: Calixte Denizet <calixte.denizet@gmail.com>
Using the `:is(a)` selector we can move the previously separate rules
into the main `.toolbarButton` override rules.
Co-authored-by: Calixte Denizet <calixte.denizet@gmail.com>
Now that we have dedicated CSS scopes for the findbar and the secondary
toolbar we have ended up with two CSS blocks with identical selectors,
so now we can combine them for simplicity and to remove some rules in
the first block that were always overridden by the second block.
Co-authored-by: Calixte Denizet <calixte.denizet@gmail.com>
We have a number of base-classes that are only intended to be extended, but never to be used directly. To help enforce this during development these base-class constructors will check for direct usage, however that code is obviously not needed in the actual builds.
*Note:* This patch reduces the size of the `gulp mozcentral` output by `~2.7` kilo-bytes, which isn't a lot but still cannot hurt.
The secondary toolbar CSS rules predate the general availability of CSS
nesting, which makes them more difficult to understand and change
safely. The primary issues are that the rules are spread over the
`viewer.css` file, they share blocks with other elements and the scope
of the rules is sometimes bigger than necessary.
This refactoring groups all remaining secondary toolbar rules together,
scoped to the top-level `#secondaryToolbar` element, for improved
overview and isolation. Note that this patch only intends to move the
existing rules around and not change any behavior.
Co-authored-by: Calixte Denizet <calixte.denizet@gmail.com>
Secondary toolbar buttons are toolbar buttons with some extra rules,
mainly to make them wider and have visible labels. However, this
similarity is currently not clearly reflected in the implementation
because the secondary toolbar buttons use a different CSS class,
`secondaryToolbarButton`, compared to the other toolbar buttons that
use the `toolbarButton` CSS class. This also causes some duplication
in the rules and requires extra care to keep the common bits for the
`secondaryToolbarButton` class in sync with the `toolbarButton` class.
Fortunately, now that we have a dedicated CSS scope for the secondary
toolbar, we can simplify this by giving all secondary toolbar buttons
the `toolbarButton` class and explicitly listing the required overrides
in the `#secondaryToolbar` scope instead. Doing so removes most of the
special-casing for secondary toolbar buttons while explicitly listing
the differences in a single place for a better overview. It also lays
the foundation for making all toolbar buttons respect the
`browser.uidensity` Firefox preference later by reducing differences.
Co-authored-by: Calixte Denizet <calixte.denizet@gmail.com>
The secondary toolbar CSS rules predate the general availability of CSS
nesting, which makes them more difficult to understand and change
safely. The primary issues are that the rules are spread over the
`viewer.css` file, they share blocks with other elements and the scope
of the rules is sometimes bigger than necessary.
This refactoring groups all CSS rules for the secondary toolbar button
container/icons together, scoped to the top-level `#secondaryToolbar`
element, for improved overview and isolation. Note that this patch only
intends to move the existing rules around and not change any behavior.
Moreover, this patch does not move the rules for the secondary toolbar
itself and the secondary toolbar buttons; those will be part of a
follow-up patch and will be easier once this is in place first.
Co-authored-by: Calixte Denizet <calixte.denizet@gmail.com>
Because of an old oversight (by me) we don't stop sidebar resizing when the browser window loses focus, which seems generally wrong and can also lead to duplicate mouse-related event listeners being registered.
There's a fair number of event listeners in the editor-code that we're currently removing "manually", by keeping references to their event handler functions.
This was necessary since we have a "global" `AbortController` that applies to all event listeners used in the editor-code, however it's now possible to combine multiple `AbortSignal`s; please see https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/any_static
Since this functionality is [fairly new](https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/any_static#browser_compatibility) the viewer will check that `AbortSignal.any()` is available before enabling the editing-functionality.
(It should hopefully be fairly straightforward, famous last words, for users to implement a polyfill to allow editing in older browsers.)
Finally, this patch also adds checks and test-only asserts to ensure that we don't add duplicate event listeners in various editor-code.
The findbar CSS rules predate the general availability of CSS nesting,
which makes them more difficult to understand and change safely. The
primary issues are that the findbar rules are spread all over the
`viewer.css` file, they share blocks with non-findbar elements and the
scope of the rules is sometimes bigger than necessary.
This refactoring groups all findbar-related CSS rules together, scoped
to the top-level `#findbar` element, for improved overview and
isolation. Note that this patch only intends to move the existing rules
around and not change any behavior yet, but it does lay the foundation
for e.g. making the findbar respect the `browser.uidensity` preference
in Firefox in follow-up work.
Co-authored-by: Calixte Denizet <calixte.denizet@gmail.com>
It looks like this has accidentally been copy/pasted from the
`Textfields and focus` block, which is the only one in which a test
actually uses it. We can therefore safely remove it from all other
blocks where no test uses it.
This commit updates the Babel plugin to:
- apply the same flattening logic that we already
have for blocks, to flatten blocks nested inside
class static blocks
- remove class static blocks when, after flattening
all the blocks they contain, they are empty.
Before this commit, the transform output was the
same as the input.
Given that we're removing event listeners with `AbortSignal` it's no longer necessary to keep a reference to a few of the event handler functions in order to remove them.
Hence we can simply inline the relevant `bind`-calls instead, which reduces the code-size a tiny bit.
The `waitForClick` helper function is functionality-wise mostly a
reduced copy of the more generic `waitForEvent` helper function that
we use for other integration tests, so we can safely replace it to
reduce the amount of code.
Moreover, the `waitForClick` code is prone to intermittent failures
given recent assertion failures we have seen on the bots (one of them
is linked in #18396) while `waitForEvent` has recently been fixed to
avoid intermittent failures, so usiong it should also get rid of the
flakiness for these integration tests.
The `options` handling, for the download-methods, was originally added in PR 16391 and became obsolete in PR 17771.
This fixes failures, in mozilla-central tests, which appeared after landing PR 18527.
Given that these event handlers are virtually identical, obviously with the exception of the name-parameter, let's reduce a little bit of code duplication.
- Shorten the names of the event listeners.
- Use `bind` to pass in the `PDFViewerApplication`-scope to the event handlers.
This makes the event handler code (a lot) less verbose, and this change is possible now that we're removing event listeners with `AbortSignal`.
- Move the GENERIC-only event listeners into the same pre-processor block.
*Note:* This patch reduces the size of the `gulp mozcentral` output by `~4.3` kilo-bytes, which isn't a lot but still cannot hurt.
Given that users fairly often report issues with unsupported browsers/environments it cannot hurt to provide a link to the relevant section in the FAQ.
We have a fair number of (effectively) single-line event handlers in the `web/app.js` file, which leads to unnecessarily verbose code. These can, without affecting readability too much, be replaced either by:
- Using `bind` for the simplest cases.
- Using arrow-functions for the remaining ones.
Note that this is possible since we started removing event listeners with `AbortSignal`, which means that we no longer need to keep a reference to the event handler functions to be able to remove them.
Given that the old event handler functions use fairly long function names, and the way that they access `PDFViewerApplication` (given their scope), they impact the overall code-size unnecessarily.
*Note:* This patch reduces the size of the `gulp mozcentral` output by `~3.7` kilo-bytes, which isn't a lot but still cannot hurt.
This patch adds a new entry in the secondary menu in order to open a dialog to let the user:
- disables the alt-text generation thanks to a ML model;
- deletes the alt-text model downloaded in Firefox;
- disabled the new alt-text flow.
Unfortunately it turns out (perhaps unsurprisingly) that even the new bug report template isn't stopping users from leaving out the single most important part, i.e. `Attach (recommended) or Link to PDF file`, despite it now being marked as a required field.
Added annotations could have some quadpoints (highlight, ink).
The isNumberArray check was returning false and consequently the annotation wasn't
printable.
The tests didn't catch this issue because the quadpoints were passed as Array.
So driver.js has been updated in order to pass them as Float32Array in order
to be in a situation similar to the real life one.
Over time a couple of event listeners have been placed in the constructor, despite there being an existing helper method for that purpose. To improve the code organization, let's move these to the intended method instead.
This refactoring lays the foundation for making the toolbar height
configurable in Firefox via the `browser.uidensity` preference. For
this to work correctly the toolbar height must be defined in a single
place that can easily be updated dynamically, hence this patch which
moves it to a CSS variable in such a way that the rest of the UI adapts
correctly if the value is changed.
Co-authored-by: Calixte Denizet <calixte.denizet@gmail.com>
The HTML button elements we use are all regular buttons that don't
submit form data to a server. According to
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#notes
those buttons should all have their type set to `button` explicitly, but
we only do that a handful of them. This commit fixes the issue by
consistently giving all our buttons the `button` type.
Co-authored-by: Calixte Denizet <calixte.denizet@gmail.com>
The sidebar and secondary toolbar both have a reference to their toggle
buttons in their own sections in `getViewerConfiguration`, so it makes
sense for the findbar to do the same.
While we actually have a findbar-specific reference to the toggle
button, I noticed that we don't use it consistently because the toolbar
also has a reference to the exact same toggle button and we use both in
the code. This is probably for historical reasons: the docstring in the
toolbar file indicates that the `viewFind` element is an input to the
component, but that option is never actually used in the code itself.
This commit fixes the issue by removing the toolbar-specific reference,
since it's not actually used (anymore) in the toolbar code, so that we
consistently use the findbar-specific reference everywhere.
This dependency got introduced in PR #10293, almost six years ago now,
because `eslint-plugin-mozilla` didn't work without it but also didn't
require it as a dependency itself.
However, nowadays `eslint-plugin-mozilla` works just fine without it,
and other dependencies that need it correctly require it themselves.
This can be seen using `npm ls globals`:
```
$ npm ls globals
pdf.js
├─┬ @babel/core@7.24.9
│ └─┬ @babel/traverse@7.25.0
│ └── globals@11.12.0
├─┬ @babel/preset-env@7.25.0
│ └─┬ @babel/plugin-transform-classes@7.25.0
│ └── globals@11.12.0
├─┬ eslint-plugin-unicorn@55.0.0
│ └── globals@15.8.0 deduped
├─┬ eslint@8.57.0
│ ├─┬ @eslint/eslintrc@2.1.4
│ │ └── globals@13.24.0
│ └── globals@13.24.0
└── globals@15.8.0
```
Further proof that `eslint-plugin-mozilla` (no longer) uses `globals` is
from a source code search in
https://searchfox.org/mozilla-central/search?q=globals&path=&case=false®exp=false.
The only results for `eslint-plugin-mozilla` refer to a file named
`globals.js`, but the `globals` NPM package is not actually imported
anywhere.
Given this we should be able to safely get rid of this explicit
dependency on our end now.
For the Firefox pdf viewer, we want to use AI to guess an alt-text when adding an image to a pdf.
For now the telemtry stuff is not implemented and will come soon.
In order to test it locally:
- set enableAltText, enableFakeMLManager and enableUpdatedAddImage to true.
or in Firefox:
- set browser.ml.enable, pdfjs.enableAltText and pdfjs.enableUpdatedAddImage to true.
This is possible thanks to features, i.e. private fields and in particular static initialization blocks, that didn't exist back when we started using classes in the code-base.
The current error-messages also mention internal parameters, which an end-user obviously don't have to care about. So, let's try to avoid confusion here by only including the API parameters.
- Initialize all user-options upfront, to allow getting the options without having to check if they exist first. (This is easier to implement after PR 18475 landed.)
- Move the user-options into a private field in `AppOptions`. (This code is old enough to pre-date general availability of that class feature.)
- Move code/methods limited to GENERIC-builds into `AppOptions`.
Currently we'll only dispatch events, for the options that support it, when updating preferences. Since this could potentially lead to inconsistent behaviour, let's avoid any future surprises by *always* dispatching events regardless of how the relevant option is being changed.
Obviously we should then also dispatch events in `AppOptions.set`, and to avoid adding even more duplicated code this method is changed into a wrapper for `AppOptions.setAll`.
While this is technically a tiny bit less efficient I don't think it matters in practice, since outside of development- and debug-mode `AppOptions.set` is barely used.
When selecting text, hovering over an element
causes all the text between (according the the dom
order) the current selection and that element to
be selected.
This means that when, while selecting, the cursor
moves over a link, all the text in the page gets
selected. Setting `user-select: none` on the link
annotations would improve the situation, but it
still makes it impossible to extend the selection
within a link without using Shift+arrows keys on
the keyboard.
This commit fixes the problem by setting
`pointer-events: none` on the `<section>`s in the
annotation layer while selecting some text. This
way, they are ignored for hit-testing and do not
affect selection.
It is still impossible to _start_ a selection
inside a link, as the link text is covered by the
link annotation.
Fixes#18266
Since "localeProperties" is needed in Firefox, let's remove a tiny bit of option duplication by using it in the GENERIC builds as well.
For convenience, the old debug-only "locale" hash-parameter is kept intact.
The `streamqueue` dependency is only used for the test targets in the
Gulpfile to make sure that the test types are run in series. This is
done by modelling the test processes as readable streams and then having
`streamqueue` combine them into a single readable stream for Gulp that
processes the inner readable streams in series (in contrast to the
`ordered-read-streams` dependency which is very similar but processes
the inner streams in parallel).
However, modelling the test processes as readable streams is a bit odd
because we're not actually streaming any data as one might expect.
Instead, we only use them to signal test process completion/abortion.
Fortunately nowadays, with modern Gulp versions, we don't need readable
streams and `streamqueue` anymore because we can achieve the same result
with simple asynchronous functions that can be passed to e.g.
`gulp.series()` calls. Note that we already do this in various places,
and overall it should be a better fit for test process invocations.
For options with varying types, see `useSystemFonts`, we're not sufficiently validating the type when setting a new value. This means that for an option that uses `OptionKind.UNDEF_ALLOWED` we'd allow *any* value, which is obviously not the intention.
Hence we instead introduce a new and *optional* `type`-field that allows specifying exactly which types are valid when multiple ones are supported.
*Note:* This obviously didn't occur to me until after PR 18465 landed, sorry about that!
This option is old enough that it predates e.g. the introduction of AppOptions, so it probably cannot hurt to re-factor this a little bit now.
- In development-mode we can just set this directly in AppOptions.
- In the extension-builds we still need to set it dynamically, however by moving this code we get the benefit of being able to avoid storing a data-URL in that case; note how [the API ignores those anyway](98e772727e/src/display/api.js (L256-L262)).
To avoid introducing any inline "hacks" in the viewer-code this meant adding `useSystemFonts` to the AppOptions, which thus required some new functionality since the default value should be `undefined` given how the option is handled in the API; note [this code](ed83d7c5e1/src/display/api.js (L298-L301)).
Finally, also moves the definition of the development-mode `window.isGECKOVIEW` property to the HTML file such that it's guaranteed to be set regardless of how and when it's accessed.
The old implementation in `PDFViewerApplication.download` means that if the `getDocument`-call hasn't yet downloaded the *entire* PDF document it will be re-downloaded. This seems generally undesirable since:
- In some (probably rare) cases a URL may not be valid an arbitrary number of times, which means that the download may fail.
- It will lead to wasted resources, since we'll end up fetching the same PDF document *twice* in that case (once via the `getDocument`-call and once to allow the user to save it).
Hence this patch suggests that we change this very old code to instead always call the `PDFDocumentProxy.getData` method, since that'll trigger immediate downloading of the remaining document via the existing `getDocument`-call.
Finally, the patch removes the `PDFViewerApplication.downloadComplete` property since it's now unused.
This was only necessary to prevent (unlikely) visual glitches when `disableAutoFetch = true` is being used.
However, it turns out that we can move this functionality into the `ProgressBar` class instead by checking if the entire PDF document has loaded. This works since the API is always reporting 100% loading progress regardless of how the document was loaded; see [this code](ed83d7c5e1/src/display/api.js (L2735-L2740)).
After the changes in PR 18413 we're now relying even more on `AppOptions` and it thus seems like a good idea to ensure that no invalid values can be added.
Hence the `AppOptions.{set, setAll}` methods will now *unconditionally* validate that the type of the values agree with the default-options.
According to the PDF specification these destinations should have a zoom parameter, which may however be `null`, but it shouldn't be omitted; please see https://opensource.adobe.com/dc-acrobat-sdk-docs/pdfstandards/PDF32000_2008.pdf#G11.2095870
Hence we try to work-around bad PDF generators by making the zoom parameter optional when validating explicit destinations in both the worker and the viewer.
To avoid having to request and await various browser data early during the PDF Viewer initialization, we can include that data in the existing preference fetching instead. With other planned changes to the PDF Viewer, the current situation would only become worse over time.
*Note:* Technically this data aren't preference-values, however we're already including other non-prefs in this list (e.g. `isInAutomation`) and doing it this way simplifies the overall implementation.
As far as I can tell `Outliner` is only exposed in the API because we need to access it when running some of the reference-tests, but is otherwise not used.
Hence this seems like something that should be kept *internal* and thus only exposed in TESTING-builds.
Switching to an editing mode can be asynchronous (e.g. if an editable annotation exists on a
visible page), so we must add a new editor only when the page rendering is done.
Somehow I managed to mess up the URL creation relevant to e.g. MOZCENTRAL builds, which is breaking the pending PDF.js update in mozilla-central; sorry about that!
To avoid future issues, we'll now always check if absolute filter-URLs are necessary regardless of the build-target.
The Git logic for pushing to the `pdfjs-dist` repository was already
removed in PR #18350, but it was forgotten to also remove the logic to
create a Git repository in the first place. This commit fixes the open
action for that from
https://github.com/mozilla/pdf.js/pull/18358#discussion_r1661367441.
Moreover, we implement the suggestion from
https://github.com/mozilla/pdf.js/pull/18358#discussion_r1661364026
about moving the Git URL to a separate variable for consistency and we
update the homepage URL to the HTTPS scheme to avoid an HTTP -> HTTPS
redirect and enforce TLS-encrypted connections.
In PR #18356 the new tab page logic was disabled to prevent Firefox
from logging failed network connections to Contile, the Mozilla Tiles
service that is used for the new tab page [1]. However, recently this
log reappeared locally and on the bots:
```
console.warn: TopSitesFeed: Failed to fetch data from Contile server:
NetworkError when attempting to fetch resource.
```
It looks like Contile communication is also triggered from other places
in Firefox such as the URL bar [2], so this commit fixes the issue by
disabling network connections to Contile [3] altogether regardless of
their origin within Firefox. Note that we don't revert the change from
PR #18356 because as noted in [4] it can't hurt to keep that disabled
too to avoid overhead for a feature we don't use in the tests.
[1] https://github.com/mozilla-services/contile
[2] 196ef8360e/browser/components/urlbar/UrlbarProviderTopSites.sys.mjs (L38)
[3] 196ef8360e/browser/components/newtab/lib/TopSitesFeed.sys.mjs (L111)
[4] https://github.com/mozilla/pdf.js/pull/18356#issuecomment-2200354730
When the mouse was hovering an existing highlight, all the text in the page
was selected.
So when the user is selecting some text or drawing a free highlight, the mouse
is disabled for the existing editors.
Because of editor z-index, the toolbar belonging to an highlight created
before a second adjacent one, can be overlapped by this new one.
So when the user select an editor we just show it on front of all the other
ones to make sure that it can be used normally.
This functionality is purposely limited to development mode and GENERIC builds, since it's unnecessary in e.g. the *built-in* Firefox PDF Viewer, and will only be used when a `<base>`-element is actually present.
*Please note:* We also have tests in mozilla-central that will *indirectly* ensure that relative filter-URLs work as intended in the Firefox PDF Viewer, see https://searchfox.org/mozilla-central/source/toolkit/components/pdfjs/test/browser_pdfjs_filters.js
---
To test that the issue is fixed, the following code can be used:
```html
<!DOCTYPE html>
<html>
<head>
<meta charset="UTF-8">
<base href=".">
<title>base href (issue 18406)</title>
</head>
<body>
<ul>
<li>Place this code in a file, named `base_href.html`, in the root of the PDF.js repository</li>
<li>Run <pre>npx gulp dist-install</pre></li>
<li>Run <pre>npx gulp server</pre></li>
<li>Open <a href="http://localhost:8888/base_href.html">http://localhost:8888/base_href.html</a> in a browser</li>
<li>Compare rendering with <a href="http://localhost:8888/web/viewer.html?file=/test/pdfs/issue16287.pdf">http://localhost:8888/web/viewer.html?file=/test/pdfs/issue16287.pdf</a></li>
</ul>
<canvas id="the-canvas" style="border: 1px solid black; direction: ltr;"></canvas>
<script src="/node_modules/pdfjs-dist/build/pdf.mjs" type="module"></script>
<script id="script" type="module">
//
// If absolute URL from the remote server is provided, configure the CORS
// header on that server.
//
const url = '/test/pdfs/issue16287.pdf';
//
// The workerSrc property shall be specified.
//
pdfjsLib.GlobalWorkerOptions.workerSrc =
'/node_modules/pdfjs-dist/build/pdf.worker.mjs';
//
// Asynchronous download PDF
//
const loadingTask = pdfjsLib.getDocument(url);
const pdf = await loadingTask.promise;
//
// Fetch the first page
//
const page = await pdf.getPage(1);
const scale = 1.5;
const viewport = page.getViewport({ scale });
// Support HiDPI-screens.
const outputScale = window.devicePixelRatio || 1;
//
// Prepare canvas using PDF page dimensions
//
const canvas = document.getElementById("the-canvas");
const context = canvas.getContext("2d");
canvas.width = Math.floor(viewport.width * outputScale);
canvas.height = Math.floor(viewport.height * outputScale);
canvas.style.width = Math.floor(viewport.width) + "px";
canvas.style.height = Math.floor(viewport.height) + "px";
const transform = outputScale !== 1
? [outputScale, 0, 0, outputScale, 0, 0]
: null;
//
// Render PDF page into canvas context
//
const renderContext = {
canvasContext: context,
transform,
viewport,
};
page.render(renderContext);
</script>
</body>
</html>
```
According to the PDF specification these destinations should have a coordinate parameter, which may however be `null`, but it shouldn't be omitted; please see https://opensource.adobe.com/dc-acrobat-sdk-docs/pdfstandards/PDF32000_2008.pdf#G11.2095870
Hence we try to work-around bad PDF generators by making the coordinate parameter optional when validating explicit destinations in both the worker and the viewer.
Given:
```css
html,
body {
height: 100%;
}
body {
display: flex;
}
```
The `<div>` appended to the `<body>` will take up the full height of the
viewport due to the implicit `align-items: stretch` of flex containers.
This results in an incorrect computed `minFontSize` value.
In the MOZCENTRAL build the `BasePreferences` class is almost unused, since it's only being used to initialize and update the `AppOptions` which means that theoretically we could move the relevant code there.
However for the other build targets (e.g. GENERIC and CHROME) we still need to keep the `BasePreferences` class, although we can re-factor things to move the necessary validation inside of `AppOptions` and thus simplify the code and reduce duplication.
The patch also moves the event dispatching, for changed preference values, into `AppOptions` instead.
Code inspection uncovered that quite a few integration tests don't wait
for scripting to be ready before proceeding, which is a source of
intermittent failures, especially on slower machines where e.g. creating
the sandbox takes longer.
This commit fixes the issues by introducing a `waitForScripting` helper
function and calling it consistently in all scripting integration tests.
Add unit test to check compatability with such cmaps
In the PDF in issue 18099. the toUnicode cmap had a line to map the glyph char codes from 00 to 7F to the corresponding code points. The syntax to map a range of char codes to a range of unicode code points is
<start_char_code> <end_char_code> <start_unicode_codepoint>
As the unicode code points are supposed to be given in UTF-16 BE, the PDF's line SHOULD have probably read
<00> <7F> <0000>
Instead it omitted two leading zeros from the UTF-16 like this
<00> <7F> <00>
This confused PDF.js into mapping these character codes to the UTF-16 characters with the corresponding HIGH bytes (01 became \u0100, 02 became \u0200, et cetera), which ended up turning latin text in the PDF into chinese when it was copied
I'm not sure if the PDF spec actually allows PDFs to do this, but since there's at least one PDF in the wild that does and other PDF readers read it correctly, PDF.js should probably support this
This unit test fails occasionally (albeit much less than before thanks
to PR #17663), so we change the parsing time check's divisor to prevent
it from happening again. If the last page's rendering time is less than
or equal to 50% of the first page's rendering time that should be enough
proof that no worker thread re-parsing occurred while also providing a
wide enough range to avoid intermittents.
Note that the assertion is now equal to the one we already have in the
"caches image resources at the document/page level, with main-thread
copying of complex images (issue 11518)" unit test which seems to work
reliably so far.
This patch fixes a situation that'll probably never happen, but nonetheless seems like something that we should address.
Currently the "updatedPreference" listener isn't registered *until after* the preferences have been read and initialized, which leaves a very short window of time where a preference change could theoretically be skipped.
If uncaught exceptions occur in the tests (which happened in #17962 and
can be triggered manually by throwing an error in `integration-boot.js`)
the teardown logic of the tests doesn't get to run and thus spawned
browser processes are not closed properly. Given that `test.mjs` is the
only process that has a reference to them they will become orphaned and
keep running if `test.mjs` exits without explicitly closing them.
This commit fixes the issue by always closing the browsers if uncaught
exceptions occur, and we make sure to log them for debugging purposes.
This integration test fails intermittently because we're not (correctly)
awaiting the character limit increase sandbox action.
For the first increase an attempt was made to handle this, but it doesn't
work correctly because the text in the field is `abcdefghijklmnopq` and
it's not be truncated until the sandbox action is completed, so because
we didn't await that we would could pass the `value !== "abcdefgh"` check
because `abcdefghijklmnopq !== abcdefgh` is true. For the second increase
we didn't have a check in place.
This commit fixes the issues by using the `waitForSandboxTrip` and
`waitForSelector` helper functions to make sure that we can only proceed
if the sandbox action is completed and the DOM element is updated.
The `renderForms` parameter pre-dates the introduction of the general `intent` parameter, which means that we're now effectively passing the same state twice to these `getOperatorList` methods.
Similar to the `mustBeViewed` method, we can check the relevant parameters within the `mustBeViewedWhenEditing` method itself since that (in my opinion) slightly helps readability of the code in the `src/core/document.js` file.
In *hindsight* this seems like a better idea, since it avoids the need to manually pass `isEditing` around as a boolean value.
Note that `RenderingIntentFlag` is *internal* functionality, not exposed in the official API, which means that it can be extended and modified as necessary.
Right now, editable annotations are using their own canvas when they're drawn, but
it induces several issues:
- if the annotation has to be composed with the page then the canvas must be correctly
composed with its parent. That means we should move the canvas under canvasWrapper
and we should extract composing info from the drawing instructions...
Currently it's the case with highlight annotations.
- we use some extra memory for those canvas even if the user will never edit them, which
the case for example when opening a pdf in Fenix.
So with this patch, all the editable annotations are drawn on the canvas. When the
user switches to editing mode, then the pages with some editable annotations are redrawn but
without them: they'll be replaced by their counterpart in the annotation editor layer.
For provenance, enabled in PR #18352, to work the repository URL in
`package.json` is required to match the repository URL of the GitHub
Actions invocation. This should fix the following error we encountered
publishing a new release today:
```
npm error 422 Unprocessable Entity - PUT https://registry.npmjs.org/pdfjs-dist - Error verifying sigstore provenance bundle: Failed to validate repository information: package.json: "repository.url" is "git+https://github.com/mozilla/pdfjs-dist.git", expected to match "https://github.com/mozilla/pdf.js" from provenance
```
It should help to have such a garbage in the logs:
```
console.warn: TopSitesFeed: Failed to fetch data from Contile server: NetworkError when attempting to fetch resource.
JavaScript error: , line 0: TypeError: NetworkError when attempting to fetch resource.
```
This PR switches from `npm install` to `npm ci` on CI. This enables some additional checks to ensure repo integrity when using CI/CD.
Read more: https://docs.npmjs.com/cli/v10/commands/npm-ci
This commit removes the following warnings from the `npm publish` output:
```
npm warn publish npm auto-corrected some errors in your package.json when publishing. Please run "npm pkg fix" to address these errors.
npm warn publish errors corrected:
npm warn publish Removed invalid "scripts"
npm warn publish "repository.url" was normalized to "git+https://github.com/mozilla/pdfjs-dist.git"
```
For the "scripts" section it turns out that if the package doesn't have
any scripts it's expected to explicitly set it to an empty object; refer
to https://github.com/npm/cli/issues/6918 and
https://github.com/denoland/dnt/pull/414.
Errors related to this `requestAnimationFrame` show up intermittently when running the integration-tests on the bots, however I've been unable to reproduce it locally.
Hence I cannot guarantee that it's enough to fix the timing issues, however this should be generally safe since the `requestAnimationFrame` invokes the `_next`-method and the first thing that one does is check that rendering hasn't been cancelled.
While the event listener is removed during testing, the `requestAnimationFrame` isn't cancelled and that occasionally shows up when the integration-tests are run on the bots.
Debugging #17931 uncovered a race condition in the way we use the
`waitForEvent` function. Currently the following happens:
1. We call `waitForEvent`, which starts execution of the function body
and immediately returns a promise.
2. We do the action that triggers the event.
3. We await the promise, which resolves if the event is triggered or
the timeout is reached.
The problem is in step 1: function body execution has started, but not
necessarily completed. Given that we don't await the promise, we
immediately trigger step 2 and it's not unlikely that the event we
trigger arrives before the event listener is actually registered in the
function body of `waitForEvent` (which is slower because it needs to be
evaluated in the page context and there is some other logic before the
actual `addEventListener` call).
This commit fixes the issue by passing the action to `waitForEvent` as
a callback so `waitForEvent` itself can call it once it's safe to do so.
This should make sure that we always register the event listener before
triggering the event, and because we shouldn't miss events anymore we
can also remove the retry logic for pasting.
The integration tests are currently not consistent in how they do
copy/pasting: some tests use the `kbCopy`/`kbPaste` functions with
waiting for the event inline, some have their own helper function to
combine those actions and some even call `kbCopy`/`kbPaste` without
waiting for the event at all (which can cause intermittent failures).
This commit fixes the issues by providing a set of four helper functions
that all tests use and that abstract e.g. waiting for the event away
from the caller. This makes the invididual tests simpler and consistent,
reduces code duplication and fixes possible intermittent failures
due to not waiting for events to trigger.
Browsers have an accessibility option that allows user to enforce
a minimum font size for all text rendered in the page, regardless
of what the font-size CSS property says. For example, it can be
found in Firefox under `font.minimum-size.x-western`.
When rendering the <span>s in the text layer, this causes the
text layer to not be aligned anymore with the underlying canvas.
While normally accessibility features should not be worked around,
in this case it is *not* improving accessibility:
- the text is transparent, so making it bigger doesn't make it more
readable
- the selection UX for users with that accessibility option enabled
is worse than for other users (it's basically unusable).
While there is tecnically no way to ignore that minimum font size,
this commit does it by multiplying all the `font-size`s in the text
layer by minFontSize, and then scaling all the `<span>`s down by
1/minFontSize.
This code contains the same bug that the previous commit fixed in
`waitForEvent`, namely that we don't clear the timeout if the event
is triggered. By using the now fixed `waitForEvent` function we not
only deduplicate this code but we also fix this issue so that no
incorrect timeout logs show up anymore.
Debugging #17931, by printing all parts of the event lifecycle including
timestamps, uncovered that some events for which a timeout was logged
actually did get triggered correctly in the browser. Going over the code
and discovering https://stackoverflow.com/questions/47107465/puppeteer-how-to-listen-to-object-events#comment117661238_65534026
showed what went wrong: if the event we wait for is triggered then
`Promise.race` resolves, but that doesn't automatically cancel the
timeout. The tests didn't fail on this because `Promise.race` resolved
correctly, but slightly later once the timeout was reached we would see
spurious log lines about timeouts for the already-triggered events.
This commit fixes the issue by canceling the timeout if the event we're
waiting for has triggered.
Currently errors in `afterAll` are logged, but don't fail the tests.
This could cause new errors during test teardown to go by unnoticed.
Moreover, the integration test use a different reporting mechanism which
also handled errors differently (this is extra reason to do #12730).
This patch fixes the issues by consistently handling errors in
`suiteStarted` and `suiteDone` in both reporting mechanisms.
Fixes#18319.
This integration test contains three issues:
- The `page.bringToFront()` call is not awaited, even though it returns
a promise (see https://pptr.dev/api/puppeteer.page.bringtofront). Note
that in other tests we do this correctly already.
- The `page.waitForSelector()` call at the end is unnecessary because
that exact condition is already checked at the end of the
`waitForImage` function we call just before this line; see
https://github.com/mozilla/pdf.js/blob/master/test/integration/stamp_editor_spec.mjs#L74.
- The pages should be closed in reversed order; please refer to the
description in #18318 for more details.
Fixes#18318.
This integration test is currently the only one that spawns a separate
browser instance. However, while it closes the browser once it's done,
it doesn't close the page (and therefore doesn't call the `testingClose`
method) like the other integration tests do.
This commit fixes this difference by closing the page before closing the
browser, thereby ensuring all regular cleanup logic gets called and we
avoid (intermittent) shutdown tracebacks in the logs. This allows
upcoming integration tests that spawn a separate browser instance to
reuse this pattern to cleanly end the test.
Given that we integrate the `closeSinglePage` code from #17962 for this
patch, @calixteman is credited as the co-author.
Co-authored-by: Calixte Denizet <calixte.denizet@gmail.com>
This commit fixes the following log line that currently shows up for
every type of tests that involve a browser:
`System addon update list error SyntaxError: XMLHttpRequest.open:
'http://%(server)s/dummy-system-addons.xml' is not a valid URL.`
If Firefox is in testing mode, the system addons update URL is
configured to a dummy URL so that it can't actually update (see for
example the same value in Marionette at
https://searchfox.org/mozilla-central/source/testing/marionette/client/marionette_driver/geckoinstance.py#109),
but this doesn't stop Firefox from trying to update, and when it does
it logs this line because the URL is obviously invalid.
Hence this patch which disables system addon updates altogether so
Firefox doesn't attempt to use the dummy URL anymore. The browser
updates are all managed by Puppeteer, and regular updates have already
been disabled too (see
6937a76f0a/packages/browsers/src/browser-data/firefox.ts (L302-L303)).
Allow apps with supportsIntegratedFind to better monitor the find state.
A recognized use case is the Firefox findbar, its "not found" sound must
consider `entireWord` and only make noise when it is off.
See related implementation in
https://hg.mozilla.org/mozilla-central/rev/16b902cbcf26
This change can help if we have to move the implementation from cpp to jsm.
Ensure that we never round the canvas dimensions above `maxCanvasPixels`
by rounding them to the preceeding multiple of the display ratio rather
than the succeeding one.
This commit introduces a test to ensure that:
- When zooming below the maxCanvasPixels limit, the canvas is rendered
with the correct size (the two sides are multiplied by the zoom factor).
- When zooming above the maxCanvasPixels limit, the canvas size is capped.
This has two advantages, as far as I'm concerned:
- The tests don't need to manually invoke multiple functions to properly clean-up, which reduces the risk of missing something.
- By collecting all the relevant clean-up in one method, rather than spreading it out, we get a much better overview of exactly what is being reset.
The release builds are currently not reproducible because ZIP files
record the modification date of files generated during the build
process, meaning that two builds from identical source code, made
at different times, result in different output.
This is undesirable because it makes detecting differences in the output
harder, for instance recently during the Gulp 5 efforts, because the
modification date differences are irrelevant and could obscure actually
important differences in the output during e.g. code changes. Moreover,
reprodicibility of build artifacts has become increasingly important;
please refer to the Reproducible Builds initiative at
https://reproducible-builds.org (note the "Why does it matter?" section
specifically) and https://reproducible-builds.org/docs/timestamps which
further explains the problem of timestamps in build artifacts.
This commit fixes the issue by configuring the ZIP file creation to use
the (fixed) date of the last Git commit for which the release is being
made. With this the build is fully reproducible so that identical source
code builds result in bit-by-bit identical output artifacts.
To improve readability we convert the compression method to take a
parameter object and use template strings where useful.
The JSDoc builds are currently not reproducible because a timestamp is
included in the output, meaning that two builds from identical source
code, made at different times, result in different output.
This is undesirable because it makes diffing the output difficult, for
instance recently during the Gulp 5 efforts, because the timestamp
differences are irrelevant and could obscure actually important
differences in the output during e.g. code changes. Moreover,
reprodicibility of build artifacts has become increasingly important;
please refer to the Reproducible Builds initiative at
https://reproducible-builds.org (note the "Why does it matter?" section
specifically) and https://reproducible-builds.org/docs/timestamps which
further explains the problem of timestamps in build artifacts.
This commit fixes the issue by configuring JSDoc to not include the
timestamps in the output. It's not relevant for end users and without it
the build is fully reproducible so that identical source code builds
result in bit-by-bit identical output artifacts.
Note that this option sadly can only be set via a configuration file,
and not via the command line parameters like we used to have, so for
consistency we also move the other options into the configuration file
so they are all in one place and the Gulpfile becomes a bit simpler.
After the previous commit this method has only a single call-site, hence we can inline the needed part of that check directly in `PDFViewerApplication.download` instead.
Currently saving a modified PDF document may fail *intermittently*, if it's triggered before the entire document has been downloaded.
When saving was originally added we only supported forms, and such PDF documents are usually small/simple enough for this issue to be difficult to trigger. However, with editing-support now available as well it's possible to modify much larger documents and this issue thus becomes easier to trigger.
One way to reproduce this issue *consistently* is to:
- Open http://localhost:8888/web/viewer.html?file=/test/pdfs/pdf.pdf#disableHistory=true&disableStream=true&disableAutoFetch=true
- Add an annotation on the first page, it doesn't matter what kind.
- Save the document.
- Open the resulting document, and notice that with the `master` branch the annotation is missing.
This should make the API documentation slightly quicker to access for
users by removing an extra click. Moreover, it makes the API
documentation blend in with the rest of the website/theme (one of the
points in #6526).
Fixes#18249.
This avoids having to repeat the same code multiple times, since besides resolving the promise we also need to send the "configure" message to the worker-thread.
The feature-testing on the worker-thread has been simplified in previous pull requests, which means that we can simplify this main-thread handler as well.
This helps reduce overall indentation in the method, thus leading to slightly less code.
Also, remove an old comment referring to Chrome 15 since that's no longer relevant now.
Wintersmith is no longer maintained given that the most recent version
is from six years ago, and all vulnerabilities that NPM reports
originate from Wintersmith's dependencies. Metalsmith, and its plugins,
on the other hand have recently had releases and don't have known
vulnerabilities. In fact, the number of reported vulnerabilities by NPM
even goes down to zero with this patch applied.
This commit therefore replaces Wintersmith with Metalsmith by providing
a transparent drop-in replacement, in a way that requires the least
amount of changes to the code and the generated output.
Note that this patch does update our versions of jQuery, Bootstrap and
the Highlight.js theme because the previous versions were very outdated
and didn't work correctly with Metalsmith. Moreover, those old versions
contained vulnerabilities that are hereby fixed.
Fixes#18198.
It's recommended to always install dependencies locally in the project
folder because global dependencies can easily conflict with other
projects and, because they are not managed by the project, diverge from
versions defined in e.g. `package.json`. Previously we installed
`gulp-cli` globally because at the time we lacked a convenient mechanism
to use Gulp otherwise, but nowadays NPM provides the `npx` command for
that purpose and recommends using it over global installations (see
https://docs.npmjs.com/downloading-and-installing-packages-globally
and PR #17489 that provided the ground work for using it).
This commit therefore updates our README and website to no longer recommend
installing `gulp-cli` globally but instead installing it locally from the
already existing entries in `package.json` like all other dependencies
we use. Not only does this remove the special-casing for `gulp-cli`
which simplifies the installation procedure, it also ensures that the
version ranges provided in `package.json` are respected.
This change is similar to the change in commit 92de2b7.
Fixes#18232.
Fixes 98ef8a1.
If for example dd:mm is failing we just try with d:m which is equivalent
to the regex /d{1,2}:m{1,2}/. This way it allows the user to forget the
0 for the first days/months.
- Use a CSS rule to display the wait-cursor during copying. Since copying may take a little while in long documents, there's a theoretical risk that something else could change the cursor in the meantime and just resetting to the saved-cursor could thus be incorrect.
- Remove the `interruptCopyCondition` listener with an AbortController, since that's slightly shorter code.
This method has only a single call-site in the viewer, since it's used as a fallback, and the functionality can be moved into the `DownloadManager.download` method instead.
There's no specification for that (even if it's possible to have an idea from
the xfa specs) so we just want to hide them in order to avoid to display something
wrong.
This helper method is simpler/shorter than it originally was[1] and with recent refactoring so is the `render`-method, hence we can just inline this code now.
---
[1] It used to e.g. dispatch the "textlayerrendered" event.
Part of this code is really old and pre-dates general availability of things such as `Blob` and `URL.createObjectURL`. To avoid having to duplicate the Blob-creation in the viewer, we can move this into `DownloadManager.download` instead.
Also, remove a couple of unnecessary `await` statements since the methods in question are synchronous.
When an image has a non-zero SMaskInData it means that the image
has an alpha channel.
With JPX images, the colorspace isn't required (by spec) so when we
don't have it, the JPX decoder will handle the conversion in RGBA
format.
This is a major version bump, and the changelog at
https://github.com/gulpjs/gulp/releases/tag/v5.0.0 indicates one
breaking change that impacts us, namely that streams are now by default
interpreted/transformed to UTF-8 encoding. This breaks `gulp.src` calls
that work on binary files such as images or CMaps, but is fortunately
easy to fix for us by disabling re-encoding for all `gulp.src` calls
(see https://github.com/gulpjs/gulp/issues/2764#issuecomment-2063415792
for more information). This restores the previous behavior of copying
the files as-is without Gulp performing any transformations to it, which
is what we want because Gulp is only used for bundling and we make sure
that the source files have the right encoding.
Instead of sending to the main thread an array of Objects for a list of points (or quadpoints),
we'll send just a basic float buffer.
It should slightly improve performances (especially when cloning the data) and use slightly less memory.
This parameter allows defining which point should remain
fixed while scaling the document. It can be used, for example,
to implement "zoom around the cursor" or "zoom around
pinch center".
The logic was previously implemented in `web/app.js`, but
moving it to the viewer scaling utilities themselves makes it
easier to implement similar zooming functionalities in
other embedders.
`updateScale` receives a `drawingDelay`, a `scaleFactor` and/or a number of `steps`.
If `scaleFactor` is a positive number different from `1` the current scale is multiplied by
that number. Otherwise, if `steps` if a positive integer the current scale is multiplied by
`DEFAULT_SCALE_DELTA` `steps` times. Finally, if `steps` is a negative integer, the
current scale is divided by `DEFAULT_SCALE_DELTA` `abs(steps)` times.
In order to share common parts between different dialogs, this patch
aims to slightly refactor the css in making it more generic.
This way it'll simplify adding a new dialog (we want to add a new one when
leaving an unsaved document).
After the re-factoring in PR 18104 there's now a *theoretical* risk that a pending `TextLayer` is never removed, which we can avoid by not registering it until `render` is invoked.
Note that this doesn't affect the viewer or tests, but if a third-party user calls `new TextLayer(...)` without a following call of either the `render`- or `cancel`-method we'd block global clean-up without this patch.
The `setTextContentSource` functionality is very old code, and was introduced years ago together with streaming of textContent.
By moving the `streamTextContent`-call into the `TextLayerBuilder` class we collect more functionality in one place and slightly reduce the amount of code needed.
Note that the referenced file is trivially corrupt, since it contains *two* PDF documents placed in the same file which doesn't make sense (and isn't how a PDF document should be updated).
However it's still a good idea to ensure that `loadFont` is able to handle errors when resolving References, since that allows us to invoke the existing fallback font handling.
This should avoid the latest JS features appearing in the TypeScript definitions, and given the currently supported browsers/environments (in PDF.js) we shouldn't need to target an even older ES-version.
Apparently we forgot to update this in the version 4 release, so better late than never I suppose.
- Use `.mjs` extensions where appropriate.
- Remove mention of the `debugger`-functionality, since that's not really relevant to users.
- Unify the whitespace handling to use spaces consistently.
- Move the definition of the `loadingParams` Object, to simplify the code.
- Add a unit-test, since none existed and the viewer depends on this functionality.
Over time the number of integration tests that get the rectangle for a
given selector has increased quite a bit, and the code to do so has
consequently become duplicated.
This commit refactors the integration tests to move the rectangle
fetching code to a single place, which reduces the code by over 400
lines and makes the individual tests simpler.
- Use `this` rather than explicitly spelling out the class-name in the static `#enableGlobalSelectionListener` method, since that leads to shorter code. Given that all the relevant static fields are *private* ESLint will catch any scope errors in the code.
- Reduce a little bit of duplication when using the `#selectionChangeAbortController` signal.
- Utilize early returns in the "selectionchange" event handler, since that reduces overall indentation which helps readability a tiny bit.
The `merge-stream` dependency is no longer maintained and doesn't work
in combination with Gulp 5 anymore (for more information refer to
https://github.com/gulpjs/gulp/issues/2802#issuecomment-2094130656).
Fortunately the Gulp team maintains a drop-in replacement dependency
called `ordered-read-streams` with the same API as `merge-stream`.
Indeed, running all affected Gulp targets and comparing build artifacts
with `diff -r <old> <new>` confirms that no unexpected changes are made.
Fixes a part of #17922.
This commit replaces most `waitForTimeout` occurrences with calls to
`waitForFunction` or `waitForSandboxTrip`. Note that the occurrences in
the "must check that focus/blur callbacks aren't called" test remain
until we find a good way to ensure that nothing happened after the tab
switches (because currently we can't be sure that nothing happens since
there is nothing to await).
This commit adds a test for 0603d1ac1843bc4098d74382beda6cc511350ccd.
Before the fix the `pagerendered` events would be fired just 2-3
milliseconds after the call to `increaseScale`/`decreaseScale`.
Fixes issue #16843.
In certain cases, the text layer was misaligned
due to a difference between the `lang` attribute
of the viewer and the canvas. This commit addresses
the problem by adding the `lang` attribute to the canvas.
The issue was caused because PDF.js uses serif/sans-serif
fonts to generate the text layer and relies on system fonts.
The difference in the `lang` attribute led to different fonts
being picked, causing the misalignment.
This also required changing the initial `charCodeToGlyphId`-data to an Object, which seems generally correct since it's consistent with existing code in the `src\core\{cff_font, type1_font}.js` files.
The `through2` dependency got introduced over four years ago in #11325 to
replace the unmaintained `gulp-transform` dependency. However, sadly the
same holds for `through2` since the last release was also four years ago.
Fortunately the `through2` dependency can trivially be replaced with the
built-in Node.js `stream.Transform` API nowadays. In fact, the `through2`
dependency mentions themselves in their README already that they are "a
tiny wrapper around Node.js streams.Transform". The `stream.Transform`
API is available in all Node.js versions we support, and in Node.js 6
already the simplified constructor approach for `stream.Transform` got
introduced to simplify creating custom stream transformers; see
https://nodejs.org/docs/latest-v6.x/api/stream.html#stream_new_stream_transform_options.
This commit therefore replaces `through2` by switching to the
`stream.Transform` API directly so we don't need any wrappers anymore.
Note that for our case the only change we have to make is to enable
object mode, see https://nodejs.org/api/stream.html#object-mode, because
we pass in `VinylFile` objects instead of e.g. regular `Buffer` objects.
I have confirmed in two ways that this is indeed a drop-in replacement:
- Running the Gulp targets that call the `transform` function and
diffing the resulting `build` folder before/after this patch, with
`diff -r build-old/ build-new/`, to ensure that there are no
unexpected changes in the output.
- Changing the Gulpfile to, instead of UTF-8, transform the files to
ASCII, and diffing the resulting `build` folder to confirm that the
transformation logic works and produces different results, such as:
```
diff build/lib/core/standard_fonts.js build-ascii/lib/core/standard_fonts.js
284c284
< t["Trinité"] = true;
---
> t["Trinit�"] = true;
```
This is very old code, and predates e.g. the introduction of JavaScript classes, which creates unnecessarily unwieldy code in the viewer.
By introducing a new `TextLayer` class in the API, similar to how e.g. the `AnnotationLayer` looks, we're able to keep most parameters on the class-instance itself. This removes the need to manually track them in the viewer, and simplifies the call-sites.
This also removes the `numTextDivs` parameter from the "textlayerrendered" event, since that's only added to support default-viewer functionality that no longer exists.
Finally we try, as far as possible, to polyfill the old `renderTextLayer` and `updateTextLayer` functions since they are exposed in the library API.
For *simple* invocations of `renderTextLayer` the behaviour should thus be the same, with only a warning printed in the console.
*Please note:* This doesn't really affect the viewer, but may affect the library API if multiple PDF documents are opened in parallel.
Since we clean-up "global" textLayer-data when destroying a PDF document, this means that other active PDFs could potentially break by invoking `cleanupTextLayer` unconditionally. Note that textLayer rendering is an asynchronous task, and we thus need to ensure those are all finished before running clean-up.
The `needle` dependency originally got introduced in #12024, almost four
years ago, to be able to use pre-built binaries for the `canvas`
dependency on macOS. However, nowadays the `needle` dependency isn't
used by `canvas` anymore, or any other package we use for that matter,
as shown by the empty NPM dependency tree:
```
$ npm ls needle
pdf.js
└── needle@3.3.1
```
Investigation showed that the `canvas` package depends on the
`node-pre-gyp` package which in turn depended on `needle` (see
https://github.com/Automattic/node-canvas/issues/1110#issuecomment-411232630),
but in version 1.0.0 of `node-pre-gyp` from three years ago the `needle`
dependency got dropped in favor of `node-fetch` (see
a74f5e367c/CHANGELOG.md (L52)).
This explains why the NPM dependency tree is empty now and proves that
we can safely get rid of this dependency now.
In Node.js 14.14.0 the `fs.rmSync` function was added that removes files
and directories. The `recursive` option is used to remove directories
and their contents, making it a drop-in replacement for the `rimraf`
dependency we use.
Given that PDF.js now requires Node.js 18+ we can be sure that this
option is available, so we can safely remove `rimraf` and reduce the
number of project dependencies.
Co-authored-by: Wojciech Maj <kontakt@wojtekmaj.pl>
I broke this accidentally in PR 18089, sorry about that!
Note that since `#processItems` is private we can no longer just "replace" the method as was done in PR 18052.
The issue from #18003 hasn't been shown to be caused by PDF.js, but it
did surface that we don't have (direct) unit test coverage for the
`BaseException` class. This made it more difficult to prove that the
`stack` property was already available on exception instances, but more
importantly it caused the CI to be green even though the suggested
change would have caused the `stack` property to disappear.
To avoid future regressions, for e.g. similar changes or a rewrite from
a closure to a proper class, this commit introduces a dedicated unit
test for `BaseException` that asserts that our exception instances
indeed expose all expected properties.
The Puppeteer update should in particular be helpful for us because it
contains improved WebDriver BiDi compatibility, a newer Chrome version
(both might help for #17962) and an official deprecation of CDP for
Firefox. Note that the latter doesn't require changes on our end because
we already use WebDriver BiDi unconditionally for Firefox since commit
4db0174. The full release notes can be found at
https://github.com/puppeteer/puppeteer/releases/tag/puppeteer-core-v22.8.0.
When seleciting on a touch screen device, whenever the finger moves to a
blank area (so over `div.textLayer` directly rather than on a `<span>`),
the selection jumps to include all the text between the beginning of the
.textLayer and the selection side that is not being moved.
The existing selection flickering fix when using the mouse cannot be
trivially re-used on mobile, because when modifying a selection on
a touchscreen device Firefox will not emit any pointer event (and
Chrome will emit them inconsistently). Instead, we have to listen to the
'selectionchange' event.
The fix is different in Firefox and Chrome:
- on Firefox, we have to make sure that, when modifying the selection,
hovering on blank areas will hover on the .endOfContent element
rather than on the .textLayer element. This is done by adjusting the
z-indexes so that .endOfContent is above .textLayer.
- on Chrome, hovering on blank areas needs to trigger hovering on an
element that is either immediately after (or immediately before,
depending on which side of the selection the user is moving) the
currently selected text. This is done by moving the .endOfContent
element around between the correct `<span>`s in the text layer.
The new anti-flickering code is also used when selecting using a mouse:
the improvement in Firefox is only observable on multi-page selection,
while in Chrome it also affects selection within a single page.
After this commit, the `z-index`es inside .textLayer are as follows:
- .endOfContent has `z-index: 0`
- everything else has `z-index: 1`
- except for .markedContent, which have `z-index: 0`
and their contents have `z-index: 1`.
`.textLayer` has an explicit `z-index: 0` to introduce a new stacking context,
so that its contents are not drawn on top of `.annotationLayer`.
- Change all possible semi-private methods into properly private ones. Note that this code is old enough to predate standard classes.
- Move the `appendText` helper function into `TextLayerRenderTask`, as a private method, to avoid having to manually pass in the scope.
- Simplify `#layoutText` by directly passing in all necessary data. This is possible after the changes PR 18052.
For images that failed to decode once we want to avoid a pointless round-trip to the main-thread, which could otherwise happen for globally cached images.
- These changes will allow a simpler way of implementing PR 17770.
- The /Lang attribute is fetched lazily, with the first `getTextContent` invocation. Given the existing worker-thread caching, this will thus only need to be done *once* per PDF document (and most PDFs don't included this data).
- This makes the /Lang attribute *directly available* in the `textLayer`, which has the following advantages:
- We don't need to block, and thus delay, overall viewer initialization on fetching it (nor pass it around throughout the viewer).
- Third-party users of the `textLayer` will automatically benefit from this, once we start actually using the /Lang attribute in PR 17770.
*Please note:* This also, importantly, means that the `text` reference-tests will then cover this code (which wouldn't otherwise have been the case).
If a user manually calls `PDFPageView.prototype.update()` with a `drawingDelay`-option then it'll always be necessary to re-call the method *without* a delay afterwards, regardless of the `maxCanvasPixels`-value (e.g. even when CSS-only zooming is used).
This only happens when it's safe to do so. The exceptions are:
- when the class extends another subclass: removing the constructor would remove the error about the missing super() call
- when there are default parameters, that could have side effects
- when there are destructured prameters, that could have side effects
- The `stopAtErrors` API option, which is the inverse of the "internal" `ignoreErrors` option, is explicitly documented as applying to *parsing* (i.e. the worker-thread) while the `FontFaceObject` class is used during rendering (i.e. the main-thread); see b6765403a1/src/display/api.js (L164-L167)
- A glyph that fails in the `FontRendererFactory`, on the worker-thread, will already cause (overall) parsing to stop when `ignoreErrors === false` hence checking the option on the main-thread as well seems redundant; see b6765403a1/src/core/evaluator.js (L4527-L4533)
- Removing this option simplifies the code, and slightly reduces the number of options that we need to handle in the main-thread code.
This avoids having to add a couple of event listeners in the viewer, when debugging is enabled, and is consistent with the existing handling of `FontInspector` and `StepperManager` in the API.
This global was only introduced to work-around problems caused by the GENERIC PDF.js build using top level await. Since that was removed in the previous commit, this global is now dead code.
This limit is currently completely non-functional, since the check happens *after* the entire textLayer has been parsed and appended to the DOM. It seems that this has been *accidentally* broken ever since the introduction of `ReadableStream` support.
The reason that this hasn't caused noticeable textLayer-related performance issues in practice is probably because we nowadays manage to coalesce the textLayer into fewer overall DOM elements, whereas years ago many PDF documents ended up with one DOM element *per* glyph.
By moving this check, and thus restoring the functionality, we're also able to remove the `render` helper function and simplify the code.
The only reason that this code still accepts `TextContent` is for backward-compatibility purposes, so we can simplify the implementation by always using a `ReadableStream` internally.
*Please note:* This removes top level await from the GENERIC builds of the PDF.js library.
Despite top level await being supported in all modern browsers/environments, note [the MDN compatibility data](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/await#browser_compatibility), it seems that many frameworks and build-tools unfortunately have trouble with it.
Hence, in order to reduce the influx of support requests regarding top level await it thus seems that we'll have to try and fix this.
Given that top level await is only needed for Node.js environments, to load packages/polyfills, we re-factor things to limit the asynchronicity to that environment.
The "best" solution, with the least likelihood of causing future problems, would probably be to await the load of Node.js packages/polyfills e.g. at the top of the `getDocument`-function. Unfortunately that doesn't work though, since that's a *synchronous* function that we cannot change without breaking "the world".
Hence we instead await the load of Node.js packages/polyfills together with the `PDFWorker` initialization, since that's the *first point* of asynchronicity during initialization/loading of a PDF document. The reason that this works is that the Node.js packages/polyfills are only needed during fetching of the PDF document respectively during rendering, neither of which can happen *until* the worker has been initialized.
Hopefully this won't cause any future problems, since looking at the history of the PDF.js project I don't believe that we've (thus far) ever needed a Node.js dependency at an earlier point.
This new pattern for accessing Node.js packages/polyfills will also require some care during development *and* importantly reviewing, to ensure that no new top level await is added in the main code-base.
This commit replaces `waitForFunction` calls that use
`document.activeElement` to wait for an element to get focus by simpler
`waitForSelector` expressions that use the `:focus` selector. Note that
we already use this in other tests, so this improves consistency too.
This commit replaces a `waitForTimeout` occurrence with an equivalent
`waitForSelector` expression, and removes two other `waitForTimeout`
occurrences that are obsolete because we already wait for an observable
event to trigger or class change to happen.
Note that the other `waitForTimeout` occurrences in this file are either
part of #17931 or remain until we find a good way to ensure that nothing
happened (because currently there is nothing we can await there).
- Check that the `filename` is actually a string, before parsing it further.
- Use proper "shadowing" in the `filename` getter.
- Add a bit more validation of the data in `pickPlatformItem`.
- Last, but not least, return both the original `filename` and the (path stripped) variant needed in the display-layer and viewer.
It fixes issues #14982 and #14724.
The main problem of upscaling a canvas is that it can induces some pixelation
(see issue #14724). So this patch is just removing the limit and as a side
effect it fixes issue #14982.
As far as I can tell, in looking different profiles (especially some memory profile)
in using the Firefox profiler, I don't see any noticeable difference in term of
memory use.
and implement then in using some SVG filters and composition.
Composing in using destination-in in order to multiply RGB components by
the alpha from the mask isn't perfect: it'd be a way better to natively have
alpha masks support, it induces some small rounding errors and consequently
computed RGB are approximatively correct.
In term of performance, it's a real improvement, for example, the pdf in
issue #17779 is now rendered in few seconds.
There are still some room for improvement, but overall it should be a way
better.
Node.js 22 was just released, and it seems like it's not compatible
with the `canvas` package. This commit pins the version on GitHub
actions to Node.js 21 as a temporary workaround.
This commit should be reverted once
https://github.com/Automattic/node-canvas/issues/2377
is fixed.
2024-04-26 19:23:16 +02:00
560 changed files with 57822 additions and 30895 deletions
Mozilla takes the security of our software seriously. If you believe you have found a security vulnerability in PDF.js, please report it to us as described below.
## Reporting security vulnerabilities
**Please don't report security vulnerabilities through public GitHub issues.**
Instead, please report security vulnerabilities in [Bugzilla](https://bugzilla.mozilla.org/enter_bug.cgi?product=Firefox&component=PDF%20Viewer&groups=firefox-core-security) and make sure that the checkbox in the "Security" section is checked so the required access controls are automatically configured:
The Mozilla security team will process the bug as described in [Mozilla's security bugs policy](https://www.mozilla.org/en-US/about/governance/policies/security-group/bugs).
[PDF.js](https://mozilla.github.io/pdf.js/) is a Portable Document Format (PDF) viewer that is built with HTML5.
@ -40,7 +40,7 @@ PDF.js is built into version 19+ of Firefox.
+ The official extension for Chrome can be installed from the [Chrome Web Store](https://chrome.google.com/webstore/detail/pdf-viewer/oemmndcbldboiebfnladdacbdfmadadm).
*This extension is maintained by [@Rob--W](https://github.com/Rob--W).*
+ Build Your Own - Get the code as explained below and issue `gulp chromium`. Then open
+ Build Your Own - Get the code as explained below and issue `npx gulp chromium`. Then open
Chrome, go to `Tools > Extension` and load the (unpackaged) extension from the
directory `build/chromium`.
@ -52,24 +52,15 @@ To get a local copy of the current code, clone it using git:
$ cd pdf.js
Next, install Node.js via the [official package](https://nodejs.org) or via
[nvm](https://github.com/creationix/nvm). You need to install the gulp package
globally (see also [gulp's getting started](https://github.com/gulpjs/gulp/tree/master/docs/getting-started)):
$ npm install -g gulp-cli@^2.3.0
If you prefer to not install `gulp-cli` globally, you have to prefix all the `gulp` commands with `npx` (for example, `npx gulp server` instead of `gulp server`).
If everything worked out, install all dependencies for PDF.js:
[nvm](https://github.com/creationix/nvm). If everything worked out, install
all dependencies for PDF.js:
$ npm install
> [!NOTE]
> On MacOS M1/M2 you may see some `node-gyp`-related errors when running `npm install`. This is because one of our dependencies, `"canvas"`, does not provide pre-built binaries for this platform and instead `npm` will try to build it from source. Please make sure to first install the necessary native dependencies using `brew`: https://github.com/Automattic/node-canvas#compiling.
Finally, you need to start a local web server as some browsers do not allow opening
PDF files using a `file://` URL. Run:
$ gulp server
$ npx gulp server
and then you can open:
@ -86,11 +77,11 @@ It is also possible to view all test PDF files on the right side by opening:
In order to bundle all `src/` files into two production scripts and build the generic
viewer, run:
$ gulp generic
$ npx gulp generic
If you need to support older browsers, run:
$ gulp generic-legacy
$ npx gulp generic-legacy
This will generate `pdf.js` and `pdf.worker.js` in the `build/generic/build/` directory (respectively `build/generic-legacy/build/`).
Both scripts are needed but only `pdf.js` needs to be included since `pdf.worker.js` will
@ -99,7 +90,7 @@ be loaded by `pdf.js`. The PDF.js files are large and should be minified for pro
## Using PDF.js in a web application
To use PDF.js in a web application you can choose to use a pre-built version of the library
or to build it from source. We supply pre-built versions for usage with NPM and Bower under
or to build it from source. We supply pre-built versions for usage with NPM under
the `pdfjs-dist` name. For more information and examples please refer to the
[wiki page](https://github.com/mozilla/pdf.js/wiki/Setup-pdf.js-in-a-website) on this subject.
@ -116,7 +107,7 @@ You can play with the PDF.js API directly from your browser using the live demos
More examples can be found in the [examples folder](https://github.com/mozilla/pdf.js/tree/master/examples/). Some of them are using the pdfjs-dist package, which can be built and installed in this repo directory via `gulp dist-install` command.
More examples can be found in the [examples folder](https://github.com/mozilla/pdf.js/tree/master/examples/). Some of them are using the pdfjs-dist package, which can be built and installed in this repo directory via `npx gulp dist-install` command.
For an introduction to the PDF.js code, check out the presentation by our
We're currently working on <ahref="draft/index.html">better API docs</a>, but the API is well documented in [api.js](https://github.com/mozilla/pdf.js/blob/master/src/display/api.js).
The generated API documentation, from the inline comments in [api.js](https://github.com/mozilla/pdf.js/blob/master/src/display/api.js), is available below.
<iframesrc="draft/index.html"title="PDF.js API documentation"></iframe>
└── package.json - package definition and dependencies
```
## Trying the Viewer
With the prebuilt or source version, open `web/viewer.html` in a browser and the test pdf should load. Note: the worker is not enabled for file:// urls, so use a server. If you're using the source build and have node, you can run `gulp server`.
With the prebuilt or source version, open `web/viewer.html` in a browser and the test pdf should load. Note: the worker is not enabled for file:// urls, so use a server. If you're using the source build and have node, you can run `npx gulp server`.
@ -9,4 +9,4 @@ In order to generate the file `openjpeg.js`:
## Licensing
[OpenJPEG](https://www.openjpeg.org/) is under [BSD 2-clause "Simplified" License](https://github.com/uclouvain/openjpeg/blob/master/LICENSE)
and [pdf.js.openjpeg](https://github.com/mozilla/pdf.js.openjpeg/) is released under [Apache 2](https://github.com/mozilla/pdf.js.openjpeg/blob/main/LICENSE) license so `openjpeg.js` is released under [Apache 2](https://github.com/mozilla/pdf.js.openjpeg/blob/main/LICENSE) license too.
and [pdf.js.openjpeg](https://github.com/mozilla/pdf.js.openjpeg/) is released under [BSD 2-clause](https://github.com/mozilla/pdf.js.openjpeg/blob/main/LICENSE) license so `openjpeg.js` is released under [BSD 2-clause](https://github.com/mozilla/pdf.js.openjpeg/blob/main/LICENSE) license too.
@ -217,9 +217,56 @@ pdfjs-web-fonts-disabled = Kijuko dit pa coc me kakube woko: pe romo tic ki dit
## Editing
## Remove button for the various kind of editor.
##
## Alt-text dialog
## Editor resizers
## This is used in an aria label to help to understand the role of the resizer.
## Color picker
## Show all highlights
## This is a toggle button to show/hide all the highlights.
## New alt-text dialog
## Group note for entire feature: Alternative text (alt text) helps when people can't see the image. This feature includes a tool to create alt text automatically using an AI model that works locally on the user's device to preserve privacy.
## Image alt-text settings
## "Annotations removed" bar
## Add a signature dialog
## Tab names
## Tab panels
## Controls
## Dialog buttons
## Main menu for adding/removing signatures
## Editor toolbar
## Edit signature description dialog
Some files were not shown because too many files have changed in this diff
Show More
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
