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

Annotation - For checkboxes, get field value from AS (if any) instead of V (bug 1722036) #14035

Merged
merged 1 commit into from
Sep 17, 2021

Conversation

calixteman
Copy link
Contributor

Copy link
Collaborator

@Snuffleupagus Snuffleupagus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me, with the comments addressed and all tests passing; thank you!

src/core/annotation.js Outdated Show resolved Hide resolved
… of V (bug 1722036)

  - it aims to fix https://bugzilla.mozilla.org/show_bug.cgi?id=1722036.
  - AS and V should share the same value for checkbox: it's at least what the specs say;
  - the pdf in the above bug opens correctly in Acrobat so it likely means that AS is chosen over V.
@calixteman
Copy link
Contributor Author

/botio test

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.241.84.105:8877/221930f80801f84/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

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

Live output at: http://54.193.163.58:8877/c51d4977d920b8e/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Failed

Full output at http://54.241.84.105:8877/221930f80801f84/output.txt

Total script time: 21.66 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: Passed
  • Regression tests: FAILED
  different ref/snapshot: 4
  different first/second rendering: 1

Image differences available at: http://54.241.84.105:8877/221930f80801f84/reftest-analyzer.html#web=eq.log

@pdfjsbot
Copy link

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/c51d4977d920b8e/output.txt

Total script time: 39.64 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: Passed
  • Regression tests: FAILED
  different ref/snapshot: 106
  different first/second rendering: 2

Image differences available at: http://54.193.163.58:8877/c51d4977d920b8e/reftest-analyzer.html#web=eq.log

@calixteman
Copy link
Contributor Author

It seems that we had a font rendering issue in last makeref:
see firefox-annotation-line-page1 for example: the space between e and i is a bit weird.

@Snuffleupagus
Copy link
Collaborator

Snuffleupagus commented Sep 17, 2021

It seems that we had a font rendering issue in last makeref:

I don't think it's our "fault", since this looks like (yet) another case where some font rendering changes were made in Firefox itself.
Looking at the pushlog, this was possibly caused by https://bugzilla.mozilla.org/show_bug.cgi?id=1730772 which has since been backed out. I was also seeing bad font rendering locally in Nightly, but with the latest update (< 5 minutes ago) it looks good again.

As far as I'm concerned, I think you can ignore those "regressions" and simply land this PR and run makeref.

@calixteman calixteman merged commit 7082ff9 into mozilla:master Sep 17, 2021
@calixteman
Copy link
Contributor Author

/botio makeref

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_makeref from @calixteman received. Current queue size: 0

Live output at: http://54.241.84.105:8877/7b0d95fcaa8f003/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_makeref from @calixteman received. Current queue size: 0

Live output at: http://54.193.163.58:8877/a372bf05e5e50af/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Failed

Full output at http://54.241.84.105:8877/7b0d95fcaa8f003/output.txt

Total script time: 17.98 mins

  • Lint: Passed
  • Make references: FAILED

@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

Full output at http://54.193.163.58:8877/a372bf05e5e50af/output.txt

Total script time: 37.11 mins

  • Lint: Passed
  • Make references: Passed
  • Check references: Passed

@calixteman
Copy link
Contributor Author

/botio-linux makeref

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_makeref from @calixteman received. Current queue size: 0

Live output at: http://54.241.84.105:8877/4cc611e7d178f0b/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/4cc611e7d178f0b/output.txt

Total script time: 19.62 mins

  • Lint: Passed
  • Make references: Passed
  • Check references: Passed

@calixteman calixteman deleted the 1722036 branch September 17, 2021 14:27
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.

3 participants