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

Improve the field value parsing for choice widgets to handle null values #12247

Merged
merged 2 commits into from
Aug 21, 2020

Conversation

timvandermeij
Copy link
Contributor

@timvandermeij timvandermeij commented Aug 19, 2020

The specification states that the field value is null if no item is selected and we didn't handle this case properly. Even though this did not break the rendering because we always convert the value to an array and the includes check in the display layer would simply not match, the field value would be [null] which is not expected and strange from an API perspective.

This commit fixes that by ensuring that we return an empty array in case the field value is null. The API therefore still always gives an array for the field value, but now the code is more specific so that the value is either an empty array or an array of strings.

Fixes something I noticed while debugging #12233 (see https://www.adobe.com/content/dam/acom/en/devnet/pdf/pdfs/PDF32000_2008.pdf#page=454&zoom=auto,-215,765 for the specification).

Slightly easier review with the ?w=1 parameter.

…d unit test

This commit follows the same pattern as another unit test in this file
and both reduces existing and future code duplication (since the next
commit will extend this test with an additional input).
…alues

The specification states that the field value is `null` if no item is
selected and we didn't handle this case properly. Even though this did
not break the rendering because we always convert the value to an array
and the `includes` check in the display layer would simply not match,
the field value would be `[null]` which is not expected and strange from
an API perspective.

This commit fixes that by ensuring that we return an empty array in
case the field value is `null`. The API therefore still always gives an
array for the field value, but now the code is more specific so that the
value is either an empty array or an array of strings.
@timvandermeij
Copy link
Contributor Author

/botio unittest

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_unittest from @timvandermeij received. Current queue size: 0

Live output at: http://54.215.176.217:8877/72c8e4379ffae4d/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_unittest from @timvandermeij received. Current queue size: 0

Live output at: http://54.67.70.0:8877/a15b5a9c41b13b5/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/a15b5a9c41b13b5/output.txt

Total script time: 3.76 mins

  • Unit Tests: Passed

@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

Full output at http://54.215.176.217:8877/72c8e4379ffae4d/output.txt

Total script time: 4.95 mins

  • Unit Tests: Passed

src/core/annotation.js Show resolved Hide resolved
src/core/annotation.js Show resolved Hide resolved
@timvandermeij
Copy link
Contributor Author

/botio test

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

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

Live output at: http://54.215.176.217:8877/f4a3bf234132852/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.67.70.0:8877/e8507c5b248510f/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Failed

Full output at http://54.67.70.0:8877/e8507c5b248510f/output.txt

Total script time: 26.99 mins

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

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

@pdfjsbot
Copy link

From: Bot.io (Windows)


Failed

Full output at http://54.215.176.217:8877/f4a3bf234132852/output.txt

Total script time: 29.22 mins

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

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

@timvandermeij timvandermeij merged commit 3c79093 into mozilla:master Aug 21, 2020
@timvandermeij timvandermeij deleted the acroform-choice-null branch August 21, 2020 21:17
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