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 - Add a missing method to XFAAttribute, to prevent breaking errors (issue 13748) #13764

Merged
merged 1 commit into from
Jul 20, 2021

Conversation

Snuffleupagus
Copy link
Collaborator

This is yet another case where I've got no idea if the patch is correct, but it does at least fix a breaking error :-)

Note how in the Binder._bindValue method, we're assuming that if a data-value exists then it'll also be possible to actually access it. For the XFAAttribute-implementation however, the second method is missing and that's what causes the breaking errors in issue #13748.

Please note that another possible way of "fixing" the error wouldn't been to simply change the exists-check to return false, and I could see that being a preferred solution.
However, the reason for submitting the current patch is that we get fewer warnings about Nodes with mis-matched types this way.

…ors (issue 13748)

*This is yet another case where I've got no idea if the patch is correct, but it does at least fix a breaking error :-)*

Note how in the [`Binder._bindValue` method](https://github.com/mozilla/pdf.js/blob/683ce66a48cd54fa48e96e1925277c134859e76c/src/core/xfa/bind.js#L92-L93), we're assuming that if a `data`-value exists then it'll also be possible to actually access it. For the `XFAAttribute`-implementation however, the second method is missing and that's what causes the breaking errors in issue 13748.

Please note that another possible way of "fixing" the error wouldn't been to simply change the exists-check to return `false`, and I could see that being a preferred solution.
However, the reason for submitting the current patch is that we get *fewer* warnings about Nodes with mis-matched types this way.
Copy link
Contributor

@calixteman calixteman left a comment

Choose a reason for hiding this comment

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

In the pdf in the issue, the failing ref is $.Naglowek.KodFormularza.kodSystemowy and the node in data tree is <KodFormularza kodSystemowy="IFT-3R (7)".../>.
So this patch is the right thing to do.
Thank you.

@Snuffleupagus
Copy link
Collaborator Author

/botio test

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

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

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

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

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

Live output at: http://3.101.106.178:8877/effed94698e133b/output.txt

@marco-c
Copy link
Contributor

marco-c commented Jul 20, 2021

Is this fixing #13748 completely?

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Failed

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

Total script time: 32.58 mins

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

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

@pdfjsbot
Copy link

From: Bot.io (Windows)


Failed

Full output at http://3.101.106.178:8877/effed94698e133b/output.txt

Total script time: 38.51 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://3.101.106.178:8877/effed94698e133b/reftest-analyzer.html#web=eq.log

@Snuffleupagus
Copy link
Collaborator Author

Is this fixing #13748 completely?

Unfortunately not, since there's a bunch of scripting in that document as well (I've updated the tags accordingly).

@Snuffleupagus Snuffleupagus merged commit 6c9b6bc into mozilla:master Jul 20, 2021
@Snuffleupagus
Copy link
Collaborator Author

/botio makeref

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.67.70.0:8877/531c126f24507ef/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_makeref from @Snuffleupagus received. Current queue size: 1

Live output at: http://3.101.106.178:8877/56dec221892879e/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/531c126f24507ef/output.txt

Total script time: 29.26 mins

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

@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

Full output at http://3.101.106.178:8877/56dec221892879e/output.txt

Total script time: 35.85 mins

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

@Snuffleupagus Snuffleupagus deleted the issue-13748 branch July 20, 2021 18:17
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.

4 participants