While this will perhaps not be perfect for *every* PDF document with mixed page orientation, based on the large number of bugs/issues seen over the years I'm however pretty convinced that it'll be an overall improvement in a majority of cases.
In order to improve things further, we'd probably need Firefox to support e.g. `@page` such that the viewer can provide better information to the print engine.
Currently, with `enablePrintAutoRotate = true` set, we're forced to loop through all the pages *twice* when checking for any landscape pages.
This seems completely unnecessary now, and using only *one* loop should be marginally more efficient in general.
Currently landscape pages are rotated *clockwise*, which for most documents feel wrong since holding the printed pages at their *left* edge causes the landscape pages to be viewed "upside down".
In general, since most documents are LTR ones, it feels more appropriate to instead rotate landscape pages *counterclockwise* for printing.
Similar to the existing `annotationsPromise` and `_jsActionsPromise` properties, the new `_xfaPromise` should obviously also be reset, since otherwise you might end up holding onto a lot of data for pages that are no longer active.
(That caching wasn't present in the original version of PR 13069, which is why I didn't spot it until now.)
- add an option to enable XFA rendering if any;
- for now, let the canvas layer: it could be useful to implement XFAF forms (embedded pdf in xml stream for the background and xfa form for the foreground);
- ui elements in template DOM are pretty close to their html counterpart so we generate a fake html DOM from template one:
- it makes easier to translate template properties to html ones;
- it makes faster the creation of the html element in the main thread.
Given that https://bugzilla.mozilla.org/show_bug.cgi?id=1699219 has enabled scripting for all Firefox-channels, it seems reasonable to simply set `enableScripting = true` unconditionally in the viewer preferences/options.
For now, this patch leaves the standalone viewer-components alone (such as e.g. `BaseViewer`), and if those are used scripting will thus have to be manually enabled (see e.g. the "simpleviewer"/"singlepageviewer" examples).
The issue that this patch fixes is extremely unlikely, but still theoretically possible, and I really should've caught this earlier.
Note how `BaseViewer.pagesPromise` will only be defined when a document is active, see below, and that if a printing event (triggered from scripting) arrives while the document is been closed there's a small chance that the promise isn't defined.
eb92ed12f2/web/base_viewer.js (L426-L428)
This extends PR 13033 slightly, with a heuristic to support corrupt PDF documents where the `LineAnnotation`s have an empty /Rect-entry. Please note that while I have no idea if this is "correct", this patch at least makes us output the same /BBox as re-saving in Adobe Reader does.
This builds on top of #13100, but this changes printing behavior intentionally
so I thought it was worth discussing separately, to improve the rendering on
test-cases like the one in https://bugzil.la/1697778.
This matches what e.g. Evince does when you print the PDF in there on an A4
printer.
We use margins to center horizontally, and flex to center vertically. The
reasoning for this is that it should have better browser support (though maybe
pdf.js no longer supports browsers without flex support?) and it's just as
simple.
@supports() is not supposed to report support for page descriptors, this is
depending on a Chromium bug, which doesn't treat as invalid:
```
<div style="size: 1pt 1pt">
```
Even though it should. That is
https://bugs.chromium.org/p/chromium/issues/detail?id=1079214
There's no need to use @supports for this. If the descriptor is not accepted it
will just be ignored.
That way, when Firefox implements @page { size }, which is in progress, it will
get the right behavior.
First, there's just no need to do something like this, this is simpler and
closer to what the screen renderer does.
Second, this causes overflow, which Firefox tries to compensate for when
fitting to page width, and fails at it. That is tracked in:
https://bugzilla.mozilla.org/show_bug.cgi?id=1698136
But this bug works around it by not causing overflow.
For modern browsers, we could avoid the duplication setting the style attribute
by using something like width: min/max-content, but this is not a big deal I
think, let me know if you'd prefer that.
Also I had to add a max-height for Chromium not to create extra pages. This
is harmless in Firefox and workarounds the Chromium bug, so so be it.
This is mostly done using `gulp lint --fix` with a few manual changes in
the following diff:
```diff
diff --git a/src/core/pattern.js b/src/core/pattern.js
index 365491ed3..eedd8b686 100644
--- a/src/core/pattern.js
+++ b/src/core/pattern.js
@@ -105,7 +105,7 @@ const Pattern = (function PatternClosure() {
return Pattern;
})();
-var Shadings = {};
+const Shadings = {};
// A small number to offset the first/last color stops so we can insert ones to
// support extend. Number.MIN_VALUE is too small and breaks the extend.
@@ -597,16 +597,15 @@ Shadings.Mesh = (function MeshClosure() {
if (!(0 <= f && f <= 3)) {
throw new FormatError("Unknown type6 flag");
}
- var i, ii;
const pi = coords.length;
- for (i = 0, ii = f !== 0 ? 8 : 12; i < ii; i++) {
+ for (let i = 0, ii = f !== 0 ? 8 : 12; i < ii; i++) {
coords.push(reader.readCoordinate());
}
const ci = colors.length;
- for (i = 0, ii = f !== 0 ? 2 : 4; i < ii; i++) {
+ for (let i = 0, ii = f !== 0 ? 2 : 4; i < ii; i++) {
colors.push(reader.readComponents());
}
- var tmp1, tmp2, tmp3, tmp4;
+ let tmp1, tmp2, tmp3, tmp4;
switch (f) {
// prettier-ignore
case 0:
@@ -729,16 +728,15 @@ Shadings.Mesh = (function MeshClosure() {
if (!(0 <= f && f <= 3)) {
throw new FormatError("Unknown type7 flag");
}
- var i, ii;
const pi = coords.length;
- for (i = 0, ii = f !== 0 ? 12 : 16; i < ii; i++) {
+ for (let i = 0, ii = f !== 0 ? 12 : 16; i < ii; i++) {
coords.push(reader.readCoordinate());
}
const ci = colors.length;
- for (i = 0, ii = f !== 0 ? 2 : 4; i < ii; i++) {
+ for (let i = 0, ii = f !== 0 ? 2 : 4; i < ii; i++) {
colors.push(reader.readComponents());
}
- var tmp1, tmp2, tmp3, tmp4;
+ let tmp1, tmp2, tmp3, tmp4;
switch (f) {
// prettier-ignore
case 0:
@@ -897,7 +895,7 @@ Shadings.Mesh = (function MeshClosure() {
decodeType4Shading(this, reader);
break;
case ShadingType.LATTICE_FORM_MESH:
- var verticesPerRow = dict.get("VerticesPerRow") | 0;
+ const verticesPerRow = dict.get("VerticesPerRow") | 0;
if (verticesPerRow < 2) {
throw new FormatError("Invalid VerticesPerRow");
}
```
As part of testing this, I've diffed the output of `gulp mozcentral` with/without this patch and the *only* difference is the incremented `version`/`build` numbers.
A significant portion of the code-base has now been converted to use `let`/`const`, rather than `var`, hence it should be possible to simply enable the ESLint `no-var` rule globally.
This way we can ensure that new code won't accidentally use `var`, and it also removes the need to manually enable the rule in various folders.
Obviously it makes sense to continue the efforts to replace `var`, but that should probably happen on a file and/or folder basis.
Please note that this patch excludes the following code:
- The `extensions/` folder, since that seemed easiest for now (and I don't know exactly what the support situation is for the Chromium-extension).
- The entire `external/` folder is ignored, since most of it's currently excluded from linting.
For the code that isn't imported from elsewhere (and should be ignored), we should probably (at some point) bring the code up to the same linting/formatting standard as the rest of the code-base.
- Various files in the `test/` folder are ignored, as necessary, since the way that a lot of this code is loaded will require some care (or perhaps larger re-factoring) when removing `var` usage.
While the JSDocs have never advertised `getDocument` as supporting Node.js `Buffer`s, that apparently doesn't stop users from passing such data structures to `getDocument`.
In theory the existing `instanceof Uint8Array` check ought to have caught Node.js `Buffer`s, however for reasons that I don't even pretend to understand that check actually passes. Hence this patch which, *only* in Node.js environments, will special-case `Buffer`s to hopefully provide a slightly better out-of-the-box behaviour in Node.js environments[1].
---
[1] Although I'm not sure that we necessarily want to advertise this in the JSDocs, given the specialized use-case.
Replace the `objectFromEntries` helper function with an `objectFromMap` one instead, and simplify the data lookup in the AnnotationStorage.getValue method
(This is something that I happened to notice while working on the previous patch.)
The code in the `examples/mobile-viewer/viewer.js` file is essentially copied from an older version of the default viewer, hence we can slightly simplify the `animationStarted` handling here.
Currently errors occurring within the `src/display/{text_layer, annotation_layer}.js` files are not being handled properly by the test-suite, and the tests simply time out rather than failing as intended.
This makes it *very* easy to accidentally overlook a certain type of errors, see e.g. https://github.com/mozilla/pdf.js/pull/13055#discussion_r589005041, which this patch will thus prevent.
