XFA subform with occur min=0 and no bound data displaying. #14218

janekotovich · 2021-11-02T05:38:58Z

Because

Subform nomin displays, even though it's subform is set to

This pull request

Added condition to catch empty forms by check

if subform.name === "nomin" && subform.occur.min === 0

And remove this empty forms via pushing it to a uselessNode array which later is removed.

Issue that this pull request solves

Closes: #14130

janekotovich · 2021-11-02T05:42:59Z

I didn't add a test to manifest.json because pdf in issue is not a link but attached file.
I ran locally test by adding occurmin0.pdf to pdfs,
on master branch:

gulp makeref

on subform_min_0 branch:

gulp browsertest
gulp unitest
gulp lint

All went well but please lemme know if you would prefer me adding test to manifest.json and to test/pdfs.
Thanks!

Snuffleupagus · 2021-11-02T07:28:23Z

I didn't add a test to manifest.json because pdf in issue is not a link but attached file.

Please note that whenever feasible, we'd like all patches to include tests (since that helps avoid regressions later on).

For those cases when a PDF document is attached to an issue, you've got two options for creating the test:

Normally you'd treat this just as a linked test-case, getting the URL by right-clicking on the attached file and using the "Copy Link" alternative in the context menu.
If the attached PDF document is small/simple, without any personally identifiable information and without anything that looks like copyrighted content, you can actually include the file directly in the PDF.js repository.

In this particular case, it looks like option two is actually feasible!
To add a standard, non-linked, test-case you can e.g. follow this procedure:

Download the file, naming it issue14130.pdf in this case, and place it in the test/pdfs/ folder locally.
From the root of your local PDF.js repository, run node test/add_test.js test/pdfs/issue14130.pdf which will take care of adding an entry to the test/pdfs/.gitignore file and also update the test/test_manifest.json file.
In this case, since you want an XFA-test, you then need to manually add a "enableXfa": true-line to the new entry in the test/test_manifest.json file.
Finally, you just git add the files mentioned above (i.e. test/pdfs/.gitignore, test/pdfs/issue14130.pdf, and test/test_manifest.json) and amend your existing commit.

janekotovich · 2021-11-02T07:55:35Z

@Snuffleupagus yep no problem at all, will do in a bit 😊
Thanks, I already knew how to add it, just wasn't sure regarding file attaching / link, so decided to ask here. Will update commit and push alltogether!

janekotovich · 2021-11-02T08:17:25Z

@Snuffleupagus oh sorry, I just noticed you've mentioned amend, but I rebased (as my previous ones). Hope its okay!

Snuffleupagus · 2021-11-02T08:25:18Z

oh sorry, I just noticed you've mentioned amend, but I rebased (as my previous ones). Hope its okay!

I really don't think it matters exactly how you do it, as long as the end result looks correct :-)

Snuffleupagus · 2021-11-02T08:25:31Z

/botio xfatest

pdfjsbot · 2021-11-02T08:25:32Z

From: Bot.io (Linux m4)

Received

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

Live output at: http://54.241.84.105:8877/9ad95d886205346/output.txt

pdfjsbot · 2021-11-02T08:25:32Z

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/86b55d56b4da552/output.txt

calixteman · 2021-11-02T08:29:54Z

src/core/xfa/bind.js

@@ -643,6 +643,9 @@ class Binder {
                : dataNode[$namespaceId];
            match = child[$data] = new XmlObject(nsId, child.name);
            if (this.emptyMerge) {
+              if (child.name === "nomin" && child.occur.min === 0) {


Could you point out where in the specifications you found that an element called nomin is useless ?
Afaik, an element with a child <occur min="0" ...> is useless when no data can be bound in this element.

Have a look on:

pdf.js/src/core/xfa/bind.js

Line 666 in 6a15973

uselessNodes.push(child);

The node is useless when match is null (nothing has been found in the data tree which matches with the child from the template tree) and min <= 0 (i.e. min===0).
But because of a bug, with the pdf in the issue, we never hit this path.
So there are several possibilities:

we've a match when we shouldn't;

we've min > 0 when we shouldn't;

we never go there because there is a continue or a return somewhere.

pdfjsbot · 2021-11-02T08:34:58Z

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/9ad95d886205346/output.txt

Total script time: 9.41 mins

Font tests: Passed
Unit tests: Passed
Integration Tests: Passed
Regression tests: FAILED

  different ref/snapshot: 2

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

pdfjsbot · 2021-11-02T08:40:10Z

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/86b55d56b4da552/output.txt

Total script time: 14.62 mins

Font tests: Passed
Unit tests: Passed
Integration Tests: FAILED
Regression tests: FAILED

  errors: 61

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

calixteman · 2021-11-03T14:18:41Z

src/core/xfa/bind.js

@@ -637,6 +637,9 @@ class Binder {
          if (!match) {
            // We're in matchTemplate mode so create a node in data to reflect
            // what we've in template.
+            if (child.occur.min === 0) {
+              uselessNodes.push(child);


The comment here doesn't apply to the added code so you should move it

you should add a comment (you can quote the specification or add a link to it) to explain why the node is useless.

once we know that the child is useless, you can then skip the code after.

Snuffleupagus · 2021-11-03T14:40:01Z

src/core/xfa/bind.js

@@ -635,6 +635,13 @@ class Binder {
            /* skipConsumed = */ this.emptyMerge
          ).next().value;
          if (!match) {
+            // If there is no match (no data) and occur.min === 0
+            // container is entirely excluded.
+            // https://www.pdfa.org/norm-refs/XFA-3_3.pdf


That document is over 1500 pages long, please link directly to the relevant section instead; I'm guessing that https://www.pdfa.org/norm-refs/XFA-3_3.pdf#G23.1904923 might be correct here?

I think this section is a bit more specific:
https://www.pdfa.org/norm-refs/XFA-3_3.pdf#G12.1428332

Snuffleupagus · 2021-11-03T14:41:43Z

src/core/xfa/bind.js

+            // If there is no match (no data) and occur.min === 0
+            // container is entirely excluded.


There seem to be a couple of words missing in the middle, how about this instead:

// If there is no match (no data) and `occur.min === 0` then // the container is entirely excluded.

calixteman · 2021-11-04T08:52:58Z

/botio xfatest

pdfjsbot · 2021-11-04T08:53:00Z

From: Bot.io (Linux m4)

Received

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

Live output at: http://54.241.84.105:8877/04b5426ce943514/output.txt

pdfjsbot · 2021-11-04T08:53:00Z

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/8b50451de688d1d/output.txt

calixteman · 2021-11-04T08:56:19Z

src/core/xfa/bind.js

+            // If there is no match (no data) and `occur.min === 0` then
+            // the container is entirely excluded.
+            // https://www.pdfa.org/norm-refs/XFA-3_3.pdf#G12.1428332
+            if (child.occur.min === 0) {


I missed that... but occur could be null here.
We already get the min info here:

pdf.js/src/core/xfa/bind.js

Line 560 in 611627f

const [min, max] = this._getOccurInfo(child);

so you should use this value instead.

pdfjsbot · 2021-11-04T09:01:51Z

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/04b5426ce943514/output.txt

Total script time: 8.84 mins

Font tests: Passed
Unit tests: FAILED
Integration Tests: Passed
Regression tests: FAILED

  errors: 40

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

pdfjsbot · 2021-11-04T09:06:11Z

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/8b50451de688d1d/output.txt

Total script time: 13.17 mins

Font tests: Passed
Unit tests: FAILED
Integration Tests: Passed
Regression tests: FAILED

  errors: 81

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

Subfrom nomin displays even though it's subform is set to <occur max=-1 min=0> If we look through specs of XFA 3.3 : https://www.pdfa.org/norm-refs/XFA-3_3.pdf - The min attribute is used when processing a form that contains data. Regardless of the data at least this number of instances is included. It is permissible to set this value to zero, in which case the container is entirely excluded if there is no data for it. However, in our case it doesn't happen, because we let our empty dataNode get through. Though by setting a clause: - eliminate unmatched data with occur min=0 we are checking our empty data and sending it to uselessNode array where at the end it gets removed;

calixteman · 2021-11-04T10:35:13Z

/botio xfatest

pdfjsbot · 2021-11-04T10:35:14Z

From: Bot.io (Linux m4)

Received

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

Live output at: http://54.241.84.105:8877/65b91f579bcac9e/output.txt

pdfjsbot · 2021-11-04T10:35:14Z

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/10cebcc8c8ec56b/output.txt

pdfjsbot · 2021-11-04T10:44:51Z

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/65b91f579bcac9e/output.txt

Total script time: 9.61 mins

Font tests: Passed
Unit tests: Passed
Integration Tests: Passed
Regression tests: FAILED

  different ref/snapshot: 1

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

calixteman · 2021-11-05T08:40:37Z

/botio-windows xfatest

calixteman · 2021-11-05T09:07:57Z

/botio-windows xfatest

calixteman · 2021-11-05T10:08:23Z

/botio-windows xfatest

pdfjsbot · 2021-11-05T10:08:25Z

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/611ed5e704d3543/output.txt

pdfjsbot · 2021-11-05T10:24:48Z

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/611ed5e704d3543/output.txt

Total script time: 16.38 mins

Font tests: Passed
Unit tests: Passed
Integration Tests: FAILED
Regression tests: FAILED

  different ref/snapshot: 2

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

calixteman · 2021-11-05T11:05:23Z

/botio-linux preview

pdfjsbot · 2021-11-05T11:05:24Z

From: Bot.io (Linux m4)

Received

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

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

pdfjsbot · 2021-11-05T11:09:46Z

From: Bot.io (Linux m4)

Success

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

Total script time: 4.35 mins

Published

Viewer: http://54.241.84.105:8877/a6b0ab3f519f1a8/web/viewer.html
Viewer (legacy): http://54.241.84.105:8877/a6b0ab3f519f1a8/legacy/web/viewer.html

calixteman

LGTM, thank you for doing this.

calixteman · 2021-11-05T11:12:50Z

/botio makeref

pdfjsbot · 2021-11-05T11:12:53Z

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/8016f4a866ff359/output.txt

pdfjsbot · 2021-11-05T11:12:53Z

From: Bot.io (Windows)

Received

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

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

pdfjsbot · 2021-11-05T11:33:56Z

From: Bot.io (Linux m4)

Success

Full output at http://54.241.84.105:8877/8016f4a866ff359/output.txt

Total script time: 21.03 mins

Lint: Passed
Make references: Passed
Check references: Passed

pdfjsbot · 2021-11-05T11:48:24Z

From: Bot.io (Windows)

Failed

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

Total script time: 30.56 mins

Lint: Passed
Make references: FAILED

calixteman · 2021-11-05T11:50:27Z

/botio-windows makeref

pdfjsbot · 2021-11-05T11:50:29Z

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/930af9bd00886d9/output.txt

pdfjsbot · 2021-11-05T12:29:31Z

From: Bot.io (Windows)

Success

Full output at http://54.193.163.58:8877/930af9bd00886d9/output.txt

Total script time: 39.03 mins

Lint: Passed
Make references: Passed
Check references: Passed

Snuffleupagus added the form-xfa label Nov 2, 2021

janekotovich force-pushed the subform_min_0 branch from c00c0e0 to 34b5125 Compare November 2, 2021 08:16

calixteman reviewed Nov 2, 2021

View reviewed changes

janekotovich force-pushed the subform_min_0 branch from 34b5125 to 7c76d8e Compare November 3, 2021 14:08

calixteman reviewed Nov 3, 2021

View reviewed changes

janekotovich force-pushed the subform_min_0 branch from 7c76d8e to 2674850 Compare November 3, 2021 14:34

Snuffleupagus reviewed Nov 3, 2021

View reviewed changes

janekotovich force-pushed the subform_min_0 branch from 2674850 to d324ed4 Compare November 4, 2021 00:26

calixteman reviewed Nov 4, 2021

View reviewed changes

janekotovich force-pushed the subform_min_0 branch from d324ed4 to 56b5023 Compare November 4, 2021 10:22

mozilla deleted a comment from pdfjsbot Nov 5, 2021

calixteman approved these changes Nov 5, 2021

View reviewed changes

calixteman merged commit e136afb into mozilla:master Nov 5, 2021

		// If there is no match (no data) and occur.min === 0
		// container is entirely excluded.

XFA subform with occur min=0 and no bound data displaying. #14218

XFA subform with occur min=0 and no bound data displaying. #14218

Conversation

janekotovich commented Nov 2, 2021 • edited Loading

Because

This pull request

Issue that this pull request solves

janekotovich commented Nov 2, 2021 • edited Loading

Snuffleupagus commented Nov 2, 2021 • edited Loading

janekotovich commented Nov 2, 2021

janekotovich commented Nov 2, 2021

Snuffleupagus commented Nov 2, 2021

Snuffleupagus commented Nov 2, 2021

pdfjsbot commented Nov 2, 2021

From: Bot.io (Linux m4)

Received

pdfjsbot commented Nov 2, 2021

From: Bot.io (Windows)

Received

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pdfjsbot commented Nov 2, 2021

From: Bot.io (Linux m4)

Failed

pdfjsbot commented Nov 2, 2021

From: Bot.io (Windows)

Failed

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

calixteman commented Nov 4, 2021

pdfjsbot commented Nov 4, 2021

From: Bot.io (Linux m4)

Received

pdfjsbot commented Nov 4, 2021

From: Bot.io (Windows)

Received

Choose a reason for hiding this comment

pdfjsbot commented Nov 4, 2021

From: Bot.io (Linux m4)

Failed

pdfjsbot commented Nov 4, 2021

From: Bot.io (Windows)

Failed

calixteman commented Nov 4, 2021

pdfjsbot commented Nov 4, 2021

From: Bot.io (Linux m4)

Received

pdfjsbot commented Nov 4, 2021

From: Bot.io (Windows)

Received

pdfjsbot commented Nov 4, 2021

From: Bot.io (Linux m4)

Failed

calixteman commented Nov 5, 2021

calixteman commented Nov 5, 2021

calixteman commented Nov 5, 2021

pdfjsbot commented Nov 5, 2021

From: Bot.io (Windows)

Received

pdfjsbot commented Nov 5, 2021

From: Bot.io (Windows)

Failed

calixteman commented Nov 5, 2021

pdfjsbot commented Nov 5, 2021

From: Bot.io (Linux m4)

Received

pdfjsbot commented Nov 5, 2021

From: Bot.io (Linux m4)

Success

Published

calixteman left a comment

Choose a reason for hiding this comment

calixteman commented Nov 5, 2021

pdfjsbot commented Nov 5, 2021

From: Bot.io (Linux m4)

Received

pdfjsbot commented Nov 5, 2021

janekotovich commented Nov 2, 2021 •

edited

Loading

janekotovich commented Nov 2, 2021 •

edited

Loading

Snuffleupagus commented Nov 2, 2021 •

edited

Loading