*It appears that this accidentally broken in PR 8775.*
Note that `PDFHistory.forward` is only used with certain named actions, and these aren't that commonly used, which ought to explain why this error managed to sneak in.
Steps to reproduce the issue (and verify the fix):
1. Navigate to e.g. http://mirrors.ctan.org/info/lshort/english/lshort.pdf
2. Click on a couple of links, or outline items, such that the history is populated with a few entries.
3. In the console, execute `PDFViewerApplication.pdfHistory.back()` one or more times, thus navigating back to a previous viewer position.
4. In the console, execute `PDFViewerApplication.pdfHistory.forward() one or more times.
At the last step above, no (forward) navigation happens with the current `master`; now compare with this patch.
Moreover, adjust one linked test case that did not conform to the
standard Web Archive URL format and adjust one linked test case because
the link was dead.
To support this, the following changes have to be made as well:
- Ignore `package-lock.json` since NPM creates it, but we should not
have it in the repository.
- Switch from `babel-preset-es2015` to `babel-preset-env` to resolve
the deprecation warning in the test logs. The latter is more recent
and flexible, but should be the same functionality-wise.
- `transform` now needs to have the `utf-8` encoding option provided.
If not given, it will call the callback with a `Buffer` object,
which results in an unhandled promise rejection since what is
returned from the callback is a string, not a `Buffer`.
Split the existing `PDFFunction` in two classes, a private `PDFFunction` and a public `PDFFunctionFactory``, and utilize the latter in `PDFDocument` to allow various code to access the methods of `PDFFunction`
*Follow-up to PR 8909.*
This requires us to pass around `pdfFunctionFactory` to quite a lot of existing code, however I don't see another way of handling this while still guaranteeing that we can access `PDFFunction` as freely as in the old code.
Please note that the patch passes all tests locally (unit, font, reference), and I *very* much hope that we have sufficient test-coverage for the code in question to catch any typos/mistakes in the re-factoring.
The `inline` parameter is passed to a number of methods/functions in `PDFImage`, despite not actually being used. Its value is never checked, nor is it ever assigned to the current `PDFImage` instance (i.e. no `this.inline = inline` exists).
Looking briefly at the history of this code, I was also unable to find a point in time where `inline` was being used.
As far as I'm concerned, `inline` does nothing more than add clutter to already very unwieldy method/function signatures, hence why I'm proposing that we just remove it.
To further simplify call-sites using `PDFImage`/`NativeImageDecoder`, a number of methods/functions are changed to take Objects rather than a bunch of (somewhat) randomly ordered parameters.
I don't have a good example at hand right know, but I recall seeing custom deployments of PDF.js that bundle a *specific* version of the `build/pdf.js` file and then set `PDFJS.workerSrc` to point to https://mozilla.github.io/pdf.js/build/pdf.worker.js.
That practice seems really bad since, besides (obviously) causing unnecessary server load, it will very quickly result in a version mismatch between the `pdf.js` and `pdf.worker.js` files in those PDF.js deployments.
Such a version mismatch could easily lead to either breaking errors, or even worse slightly inconsistent behaviour for an API call (if the API -> Worker interface changes, which does happen from time to time).
To avoid the problems described above, I'm thus proposing that we enforce that the versions of the `pdf.js` and `pdf.worker.js` files must always match.
The new `PDFSinglePageViewer` class extends the previously created abstract `BaseViewer` class.
There's *a lot* of existing functionality in `PDFViewer` that depends on all the pages being loaded and synchronously available, once the `setDocument` method has been called.
Given that initializing `PDFPageView` instances requires passing a DOM element to which the page is attached, the simplest solution I could come up with is to append all pages to a (hidden) document fragment and just swap them (one at a time) into the viewer when page switching occurs.
This patch introduces an abstract `BaseViewer` class, that the existing `PDFViewer` then extends. *Please note:* This lays the necessary foundation for the next patch.
Looking at `ColorSpace.parseToIR`, it will do one of the following things when called:
1. Return a String.
2. Return an Array.
3. Throw a `FormatError`.
4. In one case, return the result of *another* `ColorSpace.parseToIR` call.
However, under no circumstances will it ever return an `AlternateCS` instance.
Since it's often useful to understand why code, which has become unused, existed in the first place, let's grab a hard hat and a shovel and start digging through the history of this code :-)
The current condition was introduced in commit c198ec4323, in PR 794, but it was actually already obsolete by that time.
The preceeding `instanceof SeparationCS` condition predates commit a7278b7fbc, in PR 700.
That condition was originally introduced all the way back in commit 4e3f87b60c, in PR 692. However, it was made obsolete by commit 9dcefe1efc, which is included in the very same PR!
Hence we're left with the conclusion that not only has this code be unused for *almost* six years, it was basically never used at all save for a few refactoring commits that're part of PR 692.
Bug 1392647 has a PDF where the default width of the font
is 0. It draws some charcodes that don't have glyphs, but
we were wrongly using the 1000 default width for these
charcodes causing some text to be overlapping.
Yury Delendik
Brendan Dahl
Tim van der Meij
Jonas Jenwald
pixel
Lukas Drgon