Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change test-case issue4914 to an annotations test #6787

Merged
merged 1 commit into from Dec 21, 2015
Merged

Change test-case issue4914 to an annotations test #6787

merged 1 commit into from Dec 21, 2015

Conversation

Snuffleupagus
Copy link
Collaborator

This PDF file (see issue #4914) originally regressed in PR #4318, and was subsequently fixed in PR #4915.

I added the PDF file as a (linked) test-case in PR #6481, in an effort to prevent regressions. Since we at that time didn't have the necessary framework in place, in order to correctly test annotations, this almost regressed again in PR #6672 (comment).

In that PDF file, some of the annotations are both printable and hidden, and should definitely not be visible on normal display. Hence this patch, which adds the annotations flag to the manifest in order to ensure that those annotations won't be rendered when intent === 'display'.

@timvandermeij Would you mind reviewing this?

This PDF file (see issue 4914) originally regressed in PR 4318, and was subsequently fixed in PR 4915.

I added the PDF file as a (linked) test-case in PR 6481, in an effort to prevent regressions. Since we at that time didn't have the necessary framework in place, in order to correctly test annotations, this almost regressed *again* in PR #6672 (comment).

In that PDF file, some of the annotations are both printable and hidden, and should definitely *not* be visible on normal display. Hence this patch, which adds the `annotations` flag to the manifest in order to ensure that those annotations won't be rendered when `intent === 'display'`.
@Snuffleupagus
Copy link
Collaborator Author

/botio test

@pdfjsbot
Copy link

From: Bot.io (Linux)


Received

Command cmd_test from @Snuffleupagus received. Current queue size: 0

Live output at: http://107.21.233.14:8877/8e82a089253f6c3/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_test from @Snuffleupagus received. Current queue size: 0

Live output at: http://107.22.172.223:8877/237a52ac47a741b/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

Full output at http://107.22.172.223:8877/237a52ac47a741b/output.txt

Total script time: 20.26 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Regression tests: Passed

@pdfjsbot
Copy link

From: Bot.io (Linux)


Success

Full output at http://107.21.233.14:8877/8e82a089253f6c3/output.txt

Total script time: 20.75 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Regression tests: Passed

timvandermeij added a commit that referenced this pull request Dec 21, 2015
Change test-case `issue4914` to an `annotations` test
@timvandermeij timvandermeij merged commit eb557d2 into mozilla:master Dec 21, 2015
@timvandermeij
Copy link
Contributor

Good point, thanks for addressing this!

@Snuffleupagus Snuffleupagus deleted the issue-4914-test-annotations branch December 21, 2015 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants