This patch refactors the code responsible for setting the annotation's rectangle. Its goal is to:
- Actually check that the input array is actually an array, and if so, that it contains exactly four elements.
- Only call `normalizeRect` if the input array is valid, i.e., we do not call it for the default rectangle anymore.
Unit tests are provided just like with the other patches in this series.
Fixes#6106
To avoid future regressions, two new unit tests were added:
1. A new PDF based on the report from #6106, which contains an
OpenAction of type JavaScript and a string "this.print({...}".
2. An existing PDF from https://bugzil.la/1001080 (from #4698).
Although it does not matter, since we don't execute the JavaScript code,
I have also changed "print(true)" to "print({})" since the print method
takes an object (not a boolean). See "Printing PDF documents", page 62:
http://adobe.com/content/dam/Adobe/en/devnet/acrobat/pdfs/js_developer_guide.pdf
The special handling of the 'custom' scale value is only relevant for the `scaleSelect` dropdown in the standard viewer, hence I think that it should be placed in `viewer.js` instead.
Features / bug fixes in the preprocessor:
- Add word boundary after regex for preprocessor token matching.
Previously, when you mistakenly used "#ifdef" instead of "#if", the
line would be parsed as a preprocessor directive (because "#ifdef"
starts with "#if"), but without condition (because "def" does not
start with a space). Consequently, the condition would always be false
and anything between "#ifdef" and "#endif" would not be included.
- Add validation and error reporting everywhere, to aid debugging.
- Support nested comments (by accounting for the whole stack of
conditions, instead of only the current one).
- Add #elif preprocessor command. Could be used as follows:
//#if !FEATURE_ENABLED
//#error FEATURE_ENABLED must be set
//#endif
- Add #error preprocessor command.
- Add end-of-line word boundary after "-->" in the comment trimmer.
Otherwise the pattern would also match "-->" in the middle of a line,
and incorrectly convert something like "while(i-->0)" to "while(i0)".
Code health:
- Add unit tests for the preprocessor (run external/builder/test.js).
- Fix broken link to MDN (resolved to DXR).
- Refactor to use STATE_* names instead of magic numbers (the original
meaning of the numbers is preserved, with one exception).
- State 3 has been split in two states, to distinguish between being in
an #if and #else. This is needed to ensure that #else cannot be
started without an #if.
The previous regex was too greedy, it stripped a significant amount of
code when I put another file after core/murmurhash3.js. This was caused
by the fact that the license header in murmurhash3.js does not contain
"Mozilla Foundation", so the regex continued to match until my new file
(which had the standard license header containing "Mozilla Foundation").
It took a while to figure out why adding comments in worker_loader.js
caused the build to fail, because getWorkerSrcFiles did not print an
error message when it failed to parse the file. These issues have been
resolved as follows:
- Leading comments are stripped.
- The trailing comma is removed from the array.
- Errors are detected and useful error messages are printed.
Ordinarily, local files cannot be embedded in a non-local website. Until
this commit, the extension allowed websites to embed local PDF files on
non-local (e.g. http(s)) websites. This unintended feature is now
disabled, to align better with Chrome's existing security policies
(=local file:-URLs cannot be loaded in a tab unless expicitly allowed).
- Use rimraf instead of a custom removeDirSync implementation - rimraf
deals with edge cases like EPERM on Windows.
- Detect when the process exits before it was requested via stop(),
instead of running the cleanup handler.
- Add fallback for process detection when the process exits before it
was requested. On *nix systems, this is done via pkill and pgrep, on
Windows this is done via wmic.
- Add some asserts to check the preconditions of the methods, and output
some status information to aid debugging in case of failure.
I have verified that these changes work on ArchLinux and Windows XP,
using Chrome and Firefox, as follows:
1. node make unittest
2. node make unittest
3. Restart the Firefox process via the task manager as soon as possible.
4. node make unittest
5. Temporary lock a file/directory within the temporary profile
directory until the tests have finished, and then unlock the file
within 10 seconds.
In all cases, the auxilary browser processes are killed, and the
temporary profile directory is wiped.
Basic mathematics would suggest that a double negative should always become positive, but it appears that Adobe Reader simply ignores that case. Hence I think that it makes sense for us to do the same.
Fixes 6218.
After the creation of `PDFViewer`, its `_resetView` method takes care of resetting, among other things, the page number property. Hence we don't need to set `pdfViewer.currentPageNumber = 1;` here any more, and the comment is no longer accurate either.
When the parser finds a stream, it retrieves the Length from the stream
dictionary and advances the lexer to the offset as specified in Length.
If this Length is incorrect, the lexer could end up anywhere.
When the lexer gets in an invalid state, it could throw errors. For
example, in issue 6108, the lexer ends up inside the stream data. This
stream has the ASCIIHexDecode filter, so all characters are made up from
ASCII characters, and the lexer interprets it as a command token. Tokens
cannot be longer than 127 bytes, so eventually 128 bytes are consumed
and the lexer throws "Command token too long" error.
Another possible error is "Illegal character: 41" when the lexer happens
to end up at a ')' due to the length mismatch.
These problems are solved by catching lexer errors and recovering the
parser via the existing stream length detection branch.
*With this patch we're getting very close to fixing 6158.*
The only use-case for `PDFViewerApplication.updateScaleControls` is to try and avoid calling `selectScaleOption` from the `scalechange` event handler in viewer.js.
This will *only* happen when the user has manually changed the scale by using the `<select>` dropdown, which means that in reality this is just a micro optimization. Furthermore, `selectScaleOption` is only skipped for the "named" scale values (e.g. `auto`, `page-actual`, `page-fit`, `page-width`), thus further reducing the value of this code.
Also, since we're updating the scale `<select>` dropdown from an event handler, we're currently depending on the event being dispatched (and handled) completely before the next `scalechange` event. Relying on the execution order of the code in this way, even though it currently works, seems unfortunate since it *could* potentially cause the internal scale value and the UI from getting out of sync.
Xref offsets are relative to the start of the PDF data, not to the start
of the PDF file. This is clear if you look at the other code:
- In the XRef's readXRefTable and processXRefTable methods of XRef, the
offset of a xref entry is set to the bytes as given by a PDF file.
These values are always relative to the start of the PDF file (%PDF-).
- The XRef's readXRef method adds the start offset of the stream to
Xref entry's offset: "stream.pos = startXRef + stream.start".
Clearly, this line assumes that the entry offset excludes the start
offset.
However, when the PDF is parsed in recovery mode, the xref table is
filled with entries whose offset is relative to the start of the stream
rather than the PDF file. This is incorrect, and the fix is to subtract
the start offset of the stream from the entry's byte offset.
The manually created PDF file serves as a regression test. It is a valid
PDF, except:
- The integer to point to the start of the xref table and the %%EOF
trailer are missing. This will activate recovery mode in PDF.js
- Some junk was added before the start of the PDF file. This exposes the
bad offset bug.
