Add margin between radio buttons/checkboxes and their labels/strings

[Joewebsta]

Bugzilla Bug 1716806

This PR adds margin between radio buttons/checkboxes and their labels/strings.

Links to images that illustrate the changes can be found below:

• Example 1: Before | After
• Example 2: Before | After
• Example 3: Before | After
• Example 4: Before | After

Links to the original PDFs can be found below:

• PDF 1
• PDF 2
• PDF 3
• PDF 4

I don't have extensive experience creating PRs, so I'd be happy to receive any feedback you might have.

[timvandermeij]

/botio-linux preview

[timvandermeij]

/botio test

[timvandermeij]

I'll trigger a new round of tests. If the change is visible in existing test cases we don't have to do more, but otherwise we'll need to add a (linked or in-repo PDF file) test case to avoid regressions. (We have guidelines on how to do that, but we'll get to that if it actually turns out to be necessary.)

/botio test

[moz-tools-bot]

From: Bot.io (Windows)

---

Received

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

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

[moz-tools-bot]

From: Bot.io (Linux m4)

---

Received

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

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

[moz-tools-bot]

From: Bot.io (Linux m4)

---

Failed

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

Total script time: 26.92 mins

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

  different ref/snapshot: 354
  different first/second rendering: 2

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

[moz-tools-bot]

From: Bot.io (Windows)

---

Failed

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

Total script time: 41.71 mins

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

  different ref/snapshot: 177

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

[Joewebsta]

I squashed the two commits into one. Thanks for your help and patience on this.

I will keep an eye out for the test results.

[timvandermeij]

Fortunately this change is clearly visible in the existing test cases, so we don't have to add new tests, but I do see some unexpected movement. For example in firefox-xfa_bug1718741-page1 (any quite a few others) where the input fields are moved/shortened. Is that indeed unexpected, or is that how e.g. Adobe Reader also renders the PDF (that's usually our reference point for verifying changes)?

I haven't really noticed other unexpected changes, but there are a lot of reference image changes and I haven't looked through all of them yet, so it'd be good if @calixteman also has a look here given the experience with the XFA code.

[Joewebsta]

@timvandermeij I reviewed the pdfs and all the observed movement was intended.

I agree that it would be great if @calixteman had the time to review the changes though.

[calixteman]

@timvandermeij I reviewed the pdfs and all the observed movement was intended.

I agree that it would be great if @calixteman had the time to review the changes though.

Sorry but I don't see why the movement could have been intended in firefox-xfa_issue14071-page1.
In this case (and in lot of others) we have neither a button nor a checkbox.

[Joewebsta]

@calixteman

The requirements outlined in Bugzilla Bug 1716806 specify the need to add spacing between radio buttons (not regular buttons), checkboxes, and text inputs and their associated captions.

This pull request (PR) addresses the bug by targeting the captions associated with any radio button, checkbox, or text input element that is a direct descendant of an element with either the .xfaRight or .xfaLeft class. Margins are then appropriately applied to the left or right of the caption div element.

Regarding PDF firefox-xfa_issue14071:

• On Page 1:
  • A "Date of Birth" caption associated with a text input element has a right margin applied.
• On Page 2:
  • Several radio button captions have left-margin applied.
  • Several text input captions have right-margin applied.
    • Although the margin here may not be technically necessary, it is required for other forms, such as "Example 3: Before" (also linked in my initial comment above). Without altering the HTML structure, fixing Example 3 without affecting PDFs such as firefox-xfa_issue14071 is not feasible (at least, I couldn't think of a solution).
• On Pages 4 and 5:
  • Three instances of radio button captions have left-margin applied.

I did attempt to approach the implementation of my CSS styles differently, but couldn't come up with a superior solution.

If these changes are still deemed unacceptable, I will close the PR.

[calixteman]

Here's a screenshot from Acrobat:

You can see that the first red box for the year is almost fitting the area corresponding to the year.
The rendering in Firefox is almost the same but with your patch it's now different.
In this specific case there is no reason to change the width of the red box.

[calixteman]

Why do you want to close this PR ?
We just want to apply a margin with radio button/checkboxes as your PR title claims it, not with text fields.
I didn't check anything, but I'd say it's possible to write just few lines of CSS in using has selector.

[Joewebsta]

I gave the styles another go. The updated styles only affect the margin of checkboxes and radio buttons.

[timvandermeij]

Don't we need a rule for .xfaLeft as well? Moreover, in the Bugzilla ticket the margin directions are reversed, so .xfaRight sets a margin-left but here we set a margin-right; is that correct?

[Joewebsta]

The styles in the bugzilla ticket target the .xfaCaption and .xfaCaptionForCheckButton classes which leads to the unexpected html element movement we observed last week (See calixteman's comment re: xfa_issue14071.pdf).

I took an alternative approach by targeting the .xfaRadio and .xfaCheckbox classes instead and as a result had to swap the margin left/right properties.

The xfaRight styles below work correctly and are included in the PR:

.xfaRight > :is(.xfaRadio, .xfaCheckbox) {
  margin-right: 4px;
}

The xfaLeft styles below do not work correctly and were not included in the PR (I apologize, I should have explained my rationale for this). These styles once again lead to unexpected HTML element movement.

.xfaLeft > :is(.xfaRadio, .xfaCheckbox) {
  margin-left: 4px;
}

See the attached examples of incorrect element movement. The green background was add by me to highlight the element shift.

Any suggestions on how to proceed?

[calixteman]

@Joewebsta Can you share a link to the pdf you used ?

[Joewebsta]

The images above were taken from the following test pdfs:

• xfa_issue13994
• xfa_issue14150
• xfa_issue13213

Some additional examples are:

• xfa_bug1716816.pdf
• xfa_bug1716980.pdf
• xfa_bug1718735.pdf
• xfa_dhl_shipment.pdf
• xfa_hsbc_closure.pdf
• xfa_issue13597.pdf

[timvandermeij]

/botio test

[moz-tools-bot]

From: Bot.io (Linux m4)

---

Received

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

Live output at: http://54.241.84.105:8877/019065b41c17c79/output.txt

[moz-tools-bot]

From: Bot.io (Windows)

---

Received

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

Live output at: http://54.193.163.58:8877/47deb7aa1bcec96/output.txt

[calixteman]

As @timvandermeij pointed it, the xfaLeft and xfaRight are symmetric, so making a change in one property should imply a change in the other: it'd be very strange to have an asymmetry here.
Instead of moving the caption, I think the right way to do (which needs to be verified with the tests) is add some left/right margin to the input under xfaLeft /xfaRight.

[moz-tools-bot]

From: Bot.io (Linux m4)

---

Failed

Full output at http://54.241.84.105:8877/019065b41c17c79/output.txt

Total script time: 26.97 mins

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

  different ref/snapshot: 275
  different first/second rendering: 4

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

[moz-tools-bot]

From: Bot.io (Windows)

---

Failed

Full output at http://54.193.163.58:8877/47deb7aa1bcec96/output.txt

Total script time: 42.75 mins

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

  different ref/snapshot: 132

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

[Joewebsta]

I've previously tried applying margins to the checkbox and radio button inputs (more specifically the .xfaRadio and .xfaCheckbox classes, but these modifications did not yield the desired outcome.

Despite a couple of attempts, I've been unable to resolve this issue satisfactorily. @calixteman and @timvandermeij, thank you for help and patience, but I believe this bug exceeds my current ability to address.

[calixteman]

Did you try something like (and its equivalent for xfaLeft):

.xfaRight > input {
 margin-right: 4px;
}

?

[calixteman]

That said, don't give up :)

[Joewebsta]

@calixteman Thanks for your suggestion.

I applied the styles below locally, but still observed the element shifting unfortunately.

.xfaRight > input {
  margin-right: 4px;
}

.xfaLeft > input {
  margin-left: 4px;
}

[timvandermeij]

Closing since there has been no activity on this PR for over a month. However, please feel free to reopen this PR or create a new PR once the reference tests are passing locally; you can run them locally using npx gulp makeref on the master branch (to create the initial reference images) and then using npx gulp browsertest on your new branch (to compare the master reference images with your new code). Thanks!