The existing implementation of fakeRequestAnimationFrame
did not return a timer ID, so the frame could not be cancelled
if you wanted to cancel it. But if you do want to cancel it,
it needs to be cancelled through clearTimeout instead of
cancelAnimationFrame, because the timer IDs are different.
Signed-off-by: Jonathan Barnes <jbarnes@pivotal.io>
If we only invoke the bootstrap-enabled script when PdfJs.enabled is
true, then we don't need to check it again in the script.
This avoids a sync IPC call to the parent process.
It also keeps PdfJs.jsm from importing PdfjsContentUtils.jsm in the
child process until it is actually needed, which is one steps towards
not loading it until it is really needed.
pdfjschildbootstrap.js will always be run, but
pdfjschildbootstrap-enabled.js will only be run if PdfJs.enabled is
true. This will let us avoid some work in the child process in the
next patch.
This will need to be landed in the mozilla-central repository at the
same time as a change to nsBrowserGlue.js. See bug 1352218.
Refactor removing of the `zoomLayer` into a helper method, and use that in `PDFPageView.reset` to ensure that the entire `zoomLayer` is actually removed (issue 8209)
I happened to notice that the error handling wasn't that great, which I missed previously since there were no unit-tests for failure to load built-in CMap files.
Hence this patch, which improves the error handling *and* adds tests.
I found that PR 8105 unfortunately causes a *very serious* performance regression in long PDF documents where the `Pages` tree only has one level; my apologies for this!
Obviously we cannot revert that PR, since that would cause more issues than it solves. Hence it seems to me that the only viable solution here, is to add a simple `RefSetCache` to reduce the amount of redundant lookups.
Previously in PR 8105 caching was thought to be unnecessary, but as it turns out I don't think that we really have a choice in the matter any more.
See http://eslint.org/docs/rules/#ecmascript-6.
To try and enforce consistent rules and to help avoid some possible errors in ES6 code from the start, this patch adds a few basic ESLint rules.
Note that a two of the rules, `no-shadow` and `object-shorthand`, are currently disabled. While it'd certainly be nice to enable both of them, it's currently impossible since that would result in close to one thousand lint errors.
Remove unnecessary `xref` parameters from various method signatures in `PartialEvaluator`, since `this.xref` is already available in the relevant scope
For reasons I don't pretend to understand, we're passing around `xref` arguments to a bunch of methods despite `this.xref` being available in `PartialEvaluator`.
This patch is a small first small step towards cleaning up the, often unwieldy, signatures of methods in `PartialEvaluator`.
I really cannot understand why this change is necessary, since modern browsers such as Firefox and Chrome work just fine with the old code.
Hence this is patch is yet another "hack" that's needed just because IE apparently cannot just work like you'd expect.
For consistency, the Node factory used in the CMap unit-tests is changed as well.
Fixes 8193.
*My apologies for inadvertently breaking this in PR 8064; apparently we don't have any tests that cover this use-case :(*
Without this patch `getTextContent` will fail if called before `getOperatorList`, since loading of fonts during text-extraction may require fetching of built-in CMap files.
*Please note:* The `text` test added here, which uses an already existing PDF file, fails without this patch.
In core/document.js: `PDFDocument.prototype.parse` accesses a dictionary
property, which could throw if the underlying data is not yet available.
In core/obj.js: `get Catalog.prototype.metadata` calls
`stream.getBytes`, which can throw MissingDataException too when the
stream is a ChunkedStream.
Similar to other `try-catch` statements in `/core` code, we must re-throw `MissingDataException` to prevent issues with missing data during document loading.
Note that I'm not sure if/how we can test this, which is why the patch doesn't include any test(s).
Fixes 8180.
*After browsing through (a version of) the JPEG specification, see https://www.w3.org/Graphics/JPEG/itu-t81.pdf, I hope that this patch makes sense.*
Note that while issue 7828 became a problem after PR 7661, it isn't really a regression from than PR. The explanation is rather that we're now relying on `core/jpg.js` instead of the Native Image decoder in more situations than before, which thus exposed an *existing* issue in our JPEG decoder.
Another factor also seems to be that in many JPEG images, the DRI (Define Restart Interval) marker isn't present, in which case this bug won't manifest either.
According to https://www.w3.org/Graphics/JPEG/itu-t81.pdf#page=89 (at the bottom of the page):
"NOTE – The final restart interval may be smaller than the size specified by the DRI marker segment, as it includes only the number of MCUs remaining in the scan."
Furthermore, according to https://www.w3.org/Graphics/JPEG/itu-t81.pdf#page=39 (in the middle of the page):
"[...] If restart is enabled and the restart interval is defined to be Ri, each entropy-coded segment except the last one shall contain Ri MCUs. The last one shall contain whatever number of MCUs completes the scan."
Based on the above, it thus seem to me that we should simply ensure that we're not attempting to continue to parse Scan data once we've found all MCUs (Minimum Coded Unit) of the image.
Fixes 7828.
I happened to notice that some inequalities had the wrong order, and was surprised since I thought that the `yoda` rule should have caught that.
However, reading http://eslint.org/docs/rules/yoda#options a bit more closely than previously, it's quite obvious that the `onlyEquality` option does *exactly* what its name suggests. Hence I think that it makes sense to adjust the options such that only ranges are allowed instead.
There's still some work necessary if we want to be able to run (even a subset of) the API unit-tests on Travis.
However, this patch could be considered a small first step, since the relevant unit-tests will now rely on a `CanvasFactory` rather than using `document.createElement('canvas')` directly.
This patch gets rid of the only case in the code-base where we're throwing a plain `string`, rather than an `Error`, which besides better/more consistent error handling also allows us to enable the [`no-throw-literal`](http://eslint.org/docs/rules/no-throw-literal) ESLint rule.
The point of this patch is to fix a couple of inconsistencies in `viewer.html`, compared to the locale files, such that the viewer would work correctly even without the `l10n/` files present.
*Note:* Since this isn't changing any of the locale files, we should *not* need to update any of the string names.
Looking through the history of the findbar code, it seems that the `findPrevious`/`findNext` buttons have never had a `title` set (note PR 2168, which was the initial findbar implementation).
Furthermore, the `placeholder` of the `findInput` didn't agree 100% with locale file either, so this is also adjusted.
I noticed that we have a `.toolbarButton.group` CSS class, which is currently applied to some buttons in the viewer. Since it attempts to adjust the `margin-right` property, I was initially a bit puzzled as to why there wasn't different rules for LTR/RTL locales.
However, checking the viewer with the DevTools inspector, in both LTR and RTL locales, I quickly found that the rule in question is *always* being overridden by other CSS rules.
It thus seem to me that while this rule was probably useful at some point, it has been dead for years and could now be removed.
Andrew McCreight
Travis Grathwell
Tim van der Meij
Brendan Dahl
Jonas Jenwald
Yury Delendik
Mukul Mishra
Rob Wu
River Lune