Refactor LinkAnnotation slightly, improve handling of the GoToR action, and add unit-tests
#7116
Conversation
|
These unit-test failures look really weird, and I can't understand how/why they happen! |
|
Interestingly, it seems that the unit-tests now fail in the same way on master as well, see #3891 (comment) and below. |
|
Interestingly, the last test run (based on Snuffleupagus@acbd625) seems to suggest that the file |
|
Oh, I think I know what's going on here! |
| @@ -160,14 +160,20 @@ PDFJS.isExternalLinkTargetSet = isExternalLinkTargetSet; | |||
| * @param {HTMLLinkElement} link - The link element. | |||
| * @param {Object} params - An object with the properties: | |||
| * @param {string} params.url - An absolute URL. | |||
| * @param {boolean} params.newWindow - (optional) The 'NewWindow' property of | |||
There was a problem hiding this comment.
In #7126, there is a refactoring to add the 'target' attribute to params.
There was a problem hiding this comment.
Yes, as I mentioned above that PR should land first. After that, I've already planned on changing this code to use the target parameter instead :-)
|
This PR needs a rebase (and update) after the recently landed patches. |
Thanks for the reminder, fixed now! |
…he end This patch also makes sure that all URLs are converted to the correct encoding.
- Add support for the 'NewWindow' property. - Ensure that destinations are applied to the *remote* document, instead of the current one. - Handle the `F` entry being a standard string, instead of a dictionary.
We currently don't have *any* unit-tests for `LinkAnnotation`s, so it seemed a good idea to add a few. These tests are taken from various actual PDF files.
|
@timvandermeij Would you be willing to reviewing this PR? |
|
/botio-linux preview |
From: Bot.io (Linux)ReceivedCommand cmd_preview from @timvandermeij received. Current queue size: 0 Live output at: http://107.21.233.14:8877/87f2e233691d9ec/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/87f2e233691d9ec/output.txt Total script time: 1.01 mins Published |
|
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://107.22.172.223:8877/275d2e607a72c8c/output.txt |
From: Bot.io (Linux)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://107.21.233.14:8877/ac3612f3da193d5/output.txt |
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/275d2e607a72c8c/output.txt Total script time: 20.53 mins
|
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/ac3612f3da193d5/output.txt Total script time: 22.50 mins
|
|
Nice work, thanks! |
|
@timvandermeij Thank you so much for reviewing and merging this! |
Please refer to the individual commit messages for additional information.
This PR contains the majority of PR #7058, plus more unit-tests, since that should be useful even if we decide not to add support for relative URLs.
Note: Ignoring whitespace should simplify reviewing of the first patch, see Snuffleupagus@b63ef7a?w=1.
Edit: I'm assuming that we want to land PR #7126 before this one, since this PR should be much easier to rebase given its smaller smaller scope.That PR was merged, and this PR has been updated accordingly.