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

XFA - Overwrite AcroForm dictionary when saving if no datasets in XFA (bug 1720179) #13967

Merged
merged 1 commit into from
Sep 3, 2021

Conversation

calixteman
Copy link
Contributor

  • aims to fix https://bugzilla.mozilla.org/show_bug.cgi?id=1720179
  • in some pdfs the XFA array in AcroForm dictionary doesn't contain an entry for 'datasets' (which contains saved data), so basically this patch allows to overwrite the AcroForm dictionary with an updated XFA array when doing an incremental update.

In fixing this bug I found another one: #13966 and with both fix we can save the form in the above bug and opening it in Acrobat.

src/core/catalog.js Outdated Show resolved Hide resolved
let xfaDatasets = null;
let hasDatasets = false;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you actually need this separate boolean?
Can't you just check !!xfaDatasets in the src/core/writer.js code, and thus avoid the need to track closely related state twice?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In case xfaDatasets is null, I set it to a new reference after having visited the XFA array so it's no more null.
So the only way for xfaDatasets to be null is to not have an array for XFA.
So when we have an array we must know if the reference we've has been newly created (and in this case we must update AcroForm) or not.

We could probably get rid of this boolean in moving https://github.com/mozilla/pdf.js/blob/master/src/core/worker.js#L650 in writer.js because the afaik the xref for the xref table must be the last one (so it's why I get the new ref for xfaDatasets before calling incrementalUpdate), but I guess it's better to do that in a follow-up.

src/core/writer.js Outdated Show resolved Hide resolved
src/core/writer.js Show resolved Hide resolved
… (bug 1720179)

  - aims to fix https://bugzilla.mozilla.org/show_bug.cgi?id=1720179
  - in some pdfs the XFA array in AcroForm dictionary doesn't contain an entry for 'datasets' (which contains saved data), so basically this patch allows to overwrite the AcroForm dictionary with an updated XFA array when doing an incremental update.
@brendandahl
Copy link
Contributor

/botio unittest

@pdfjsbot
Copy link

pdfjsbot commented Sep 3, 2021

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.241.84.105:8877/399d63079302a73/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Sep 3, 2021

From: Bot.io (Windows)


Received

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

Live output at: http://54.193.163.58:8877/7ac1125b782d505/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Sep 3, 2021

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/399d63079302a73/output.txt

Total script time: 2.88 mins

  • Unit Tests: Passed

@pdfjsbot
Copy link

pdfjsbot commented Sep 3, 2021

From: Bot.io (Windows)


Success

Full output at http://54.193.163.58:8877/7ac1125b782d505/output.txt

Total script time: 5.76 mins

  • Unit Tests: Passed

@brendandahl brendandahl merged commit d9d3115 into mozilla:master Sep 3, 2021
@timvandermeij timvandermeij removed the request for review from brendandahl September 4, 2021 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants