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

[JS] Fix few bugs present in the pdf for issue #14862 #14869

Merged
merged 2 commits into from
May 3, 2022

Conversation

calixteman
Copy link
Contributor

  • since resetForm function reset a field value a calculateNow is consequently triggered.
    But the calculate callback can itself call resetForm, hence an infinite recursive loop.
    So basically, prevent calculeNow to be triggered by itself.
  • in Firefox, the letters entered in some fields were duplicated: "AaBb" instead of "AB".
    It was mainly because beforeInput was triggering a Keystroke which was itself triggering
    an input value update and then the input event was triggered.
    So in order to avoid that, beforeInput calls preventDefault and then it's up to the JS to
    handle the event.
  • fields have a property valueAsString which returns the value as a string. In the
    implementation it was wrongly used to store the formatted value of a field (2€ when the user
    entered 2). So this patch implements correctly valueAsString.
  • non-rendered fields can be updated in using JS but when they're, they must take some properties
    in the annotationStorage. It was implemented for field values, but it wasn't for
    display, colors, ...
  • it fixes Too much recursion in a form with some embedded JS #14862 and Set Textfield-Value with Javascript not working as expected #14705.

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 fixed and all tests passing; thank you!

src/display/annotation_storage.js Outdated Show resolved Hide resolved
src/scripting_api/event.js Show resolved Hide resolved
src/scripting_api/doc.js Outdated Show resolved Hide resolved
test/integration/scripting_spec.js Outdated Show resolved Hide resolved
Comment on lines 6430 to 6444
},
{ "id": "issue14862",
"file": "pdfs/issue14862.pdf",
"md5": "e64d7f2dac11e124971f1901ba59cf9c",
"rounds": 1,
"link": false,
"type": "other"
},
{ "id": "issue14705",
"file": "pdfs/issue14705.pdf",
"md5": "4f203d3e60bde33103a92e24d7e5eb73",
"rounds": 1,
"link": false,
"type": "other"
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove this, since it's not necessary when you're adding the test-cases directly to the repository.

Note that the purpose of "other" was just to allow using linked test-cases for unit/integration-tests without also needing to have them as reference tests. Hence an entry in this file is only needed in the "link": true, case.

src/display/annotation_layer.js Show resolved Hide resolved
src/display/annotation_layer.js Outdated Show resolved Hide resolved
src/display/annotation_layer.js Show resolved Hide resolved
src/display/annotation_layer.js Outdated Show resolved Hide resolved
@calixteman
Copy link
Contributor Author

/botio unittest

@pdfjsbot
Copy link

pdfjsbot commented May 3, 2022

From: Bot.io (Linux m4)


Received

Command cmd_unittest from @calixteman received. Current queue size: 1

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

@pdfjsbot
Copy link

pdfjsbot commented May 3, 2022

From: Bot.io (Windows)


Received

Command cmd_unittest from @calixteman received. Current queue size: 1

Live output at: http://54.193.163.58:8877/1aeb25dc2e5fd08/output.txt

@pdfjsbot
Copy link

pdfjsbot commented May 3, 2022

From: Bot.io (Linux m4)


Failed

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

Total script time: 3.50 mins

  • Unit Tests: FAILED

@pdfjsbot
Copy link

pdfjsbot commented May 3, 2022

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/1aeb25dc2e5fd08/output.txt

Total script time: 6.26 mins

  • Unit Tests: FAILED

@calixteman
Copy link
Contributor Author

/botio unittest

@pdfjsbot
Copy link

pdfjsbot commented May 3, 2022

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.241.84.105:8877/dd8144ecbd65ea5/output.txt

@pdfjsbot
Copy link

pdfjsbot commented May 3, 2022

From: Bot.io (Windows)


Received

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

Live output at: http://54.193.163.58:8877/574e1a7c83b63e7/output.txt

@pdfjsbot
Copy link

pdfjsbot commented May 3, 2022

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/dd8144ecbd65ea5/output.txt

Total script time: 3.27 mins

  • Unit Tests: Passed

@calixteman
Copy link
Contributor Author

/botio integrationtest

@pdfjsbot
Copy link

pdfjsbot commented May 3, 2022

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.241.84.105:8877/a3e46fd96e54bae/output.txt

@pdfjsbot
Copy link

pdfjsbot commented May 3, 2022

From: Bot.io (Windows)


Received

Command cmd_integrationtest from @calixteman received. Current queue size: 1

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

src/scripting_api/doc.js Outdated Show resolved Hide resolved
@pdfjsbot
Copy link

pdfjsbot commented May 3, 2022

From: Bot.io (Windows)


Success

Full output at http://54.193.163.58:8877/574e1a7c83b63e7/output.txt

Total script time: 6.33 mins

  • Unit Tests: Passed

@pdfjsbot
Copy link

pdfjsbot commented May 3, 2022

From: Bot.io (Linux m4)


Failed

Full output at http://54.241.84.105:8877/a3e46fd96e54bae/output.txt

Total script time: 4.69 mins

  • Integration Tests: FAILED

@pdfjsbot
Copy link

pdfjsbot commented May 3, 2022

From: Bot.io (Windows)


Failed

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

Total script time: 7.57 mins

  • Integration Tests: FAILED

- since resetForm function reset a field value a calculateNow is consequently triggered.
  But the calculate callback can itself call resetForm, hence an infinite recursive loop.
  So basically, prevent calculeNow to be triggered by itself.
- in Firefox, the letters entered in some fields were duplicated: "AaBb" instead of "AB".
  It was mainly because beforeInput was triggering a Keystroke which was itself triggering
  an input value update and then the input event was triggered.
  So in order to avoid that, beforeInput calls preventDefault and then it's up to the JS to
  handle the event.
- fields have a property valueAsString which returns the value as a string. In the
  implementation it was wrongly used to store the formatted value of a field (2€ when the user
  entered 2). So this patch implements correctly valueAsString.
- non-rendered fields can be updated in using JS but when they're, they must take some properties
  in the annotationStorage. It was implemented for field values, but it wasn't for
  display, colors, ...
- it fixes mozilla#14862 and mozilla#14705.
@Snuffleupagus
Copy link
Collaborator

Also, it seems that the Interaction in 160F-2019.pdf must reset all test is timing out consistently now.

@calixteman
Copy link
Contributor Author

Also, it seems that the Interaction in 160F-2019.pdf must reset all test is timing out consistently now.

It's very likely because of the error you caught (the typeof thing).

@calixteman
Copy link
Contributor Author

/botio integrationtest

@pdfjsbot
Copy link

pdfjsbot commented May 3, 2022

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.241.84.105:8877/bd7fc703d48e803/output.txt

@pdfjsbot
Copy link

pdfjsbot commented May 3, 2022

From: Bot.io (Windows)


Received

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

Live output at: http://54.193.163.58:8877/555057dcb6e1bc2/output.txt

@pdfjsbot
Copy link

pdfjsbot commented May 3, 2022

From: Bot.io (Linux m4)


Failed

Full output at http://54.241.84.105:8877/bd7fc703d48e803/output.txt

Total script time: 4.27 mins

  • Integration Tests: FAILED

@pdfjsbot
Copy link

pdfjsbot commented May 3, 2022

From: Bot.io (Windows)


Success

Full output at http://54.193.163.58:8877/555057dcb6e1bc2/output.txt

Total script time: 7.62 mins

  • Integration Tests: Passed

@calixteman
Copy link
Contributor Author

/botio unittest

@pdfjsbot
Copy link

pdfjsbot commented May 3, 2022

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.241.84.105:8877/eb696ff51178cdd/output.txt

@pdfjsbot
Copy link

pdfjsbot commented May 3, 2022

From: Bot.io (Windows)


Received

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

Live output at: http://54.193.163.58:8877/6264bb94f3e52a6/output.txt

@pdfjsbot
Copy link

pdfjsbot commented May 3, 2022

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/eb696ff51178cdd/output.txt

Total script time: 3.23 mins

  • Unit Tests: Passed

@pdfjsbot
Copy link

pdfjsbot commented May 3, 2022

From: Bot.io (Windows)


Success

Full output at http://54.193.163.58:8877/6264bb94f3e52a6/output.txt

Total script time: 6.55 mins

  • Unit Tests: Passed

@calixteman
Copy link
Contributor Author

/botio integrationtest

@pdfjsbot
Copy link

pdfjsbot commented May 3, 2022

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.241.84.105:8877/e4ac8f9a3d97084/output.txt

@pdfjsbot
Copy link

pdfjsbot commented May 3, 2022

From: Bot.io (Windows)


Received

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

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

@pdfjsbot
Copy link

pdfjsbot commented May 3, 2022

From: Bot.io (Linux m4)


Failed

Full output at http://54.241.84.105:8877/e4ac8f9a3d97084/output.txt

Total script time: 4.33 mins

  • Integration Tests: FAILED

@pdfjsbot
Copy link

pdfjsbot commented May 3, 2022

From: Bot.io (Windows)


Success

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

Total script time: 7.95 mins

  • Integration Tests: Passed

@calixteman
Copy link
Contributor Author

@Snuffleupagus, are you still ok with this change ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants