Interactive forms: render button widget annotations (checkboxes and radio buttons) #7898

timvandermeij · 2016-12-15T23:14:33Z

Replaces #7664.
Fixes a part of #7613.

The reference tests do not display selection marks in fields only when checkbox/radio button styling is supported by the browser because the rasterizer cannot handle that. This is not a problem for landing this patch, but we will add it to the tracking issue when this is merged.

yurydelendik

Looks good

yurydelendik · 2016-12-15T23:46:21Z

src/display/annotation_layer.js

+      }
+      this.container.appendChild(inputElement);
+
+      var labelElement = document.createElement('label');


Can you add a comment that will say that label will be visible as CSS styling will take care of it?

Done in the new commit.

yurydelendik · 2016-12-16T00:07:08Z

src/core/annotation.js

+        return Promise.resolve(operatorList);
+      }
+
+      if (this.appearance) {


Add a TODO since we need to select AS based on value.

This is already taken care of by the getDefaultAppearance function based on the AS dictionary entry. Refactoring this is already part of the tracking issue.

Snuffleupagus

Given that https://bugzilla.mozilla.org/show_bug.cgi?id=418833 has now (finally) landed, most modern browsers should now support proper styling of Checkboxes and Radio buttons.
With that in mind: Would it be possible to use normal HTML elements, instead of the label hacks used here, since I believe that that would simplify the HTML, CSS and JavaScript a great deal here?

Yes this would mean that these elements wouldn't look as nice in browsers without support for styling them, but given the simplicity of that implementation I'd greatly prefer the approach with proper form elements!

Snuffleupagus · 2016-12-16T09:33:15Z

web/annotation_layer_builder.css

@@ -97,6 +99,66 @@
  width: 115%;
 }

+.annotationLayer .buttonWidgetAnnotation.checkBox label,
+.annotationLayer .buttonWidgetAnnotation.radioButton label {


Unless I'm mistaken, most of these rules are the same as those used above in

pdf.js/web/annotation_layer_builder.css

Line 44 in e3d7ac3

.annotationLayer .textWidgetAnnotation input,

, hence moving these rules there instead (with possible necessary overrides here) might make the code more readable and maintainable!?

Done in the new commit.

Snuffleupagus · 2016-12-16T09:34:29Z

test/annotation_layer_test.css

@@ -75,6 +77,26 @@
  padding-right: 0;
 }

+.annotationLayer .buttonWidgetAnnotation.checkBox label,
+.annotationLayer .buttonWidgetAnnotation.radioButton label {


Unless I'm mistaken, most of these rules are the same as those used above in https://github.com/timvandermeij/pdf.js/blob/e3d7ac3595cf4a0832ba60835a0302e9e117d6df/test/annotation_layer_test.css#L46, hence moving these rules there instead (with possible necessary overrides here) might make the code more readable and maintainable!?

Done in the new commit.

timvandermeij · 2016-12-16T22:05:28Z

/botio-linux preview

pdfjsbot · 2016-12-16T22:05:29Z

From: Bot.io (Linux)

Received

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

Live output at: http://107.21.233.14:8877/961fde53b838aef/output.txt

pdfjsbot · 2016-12-16T22:07:41Z

From: Bot.io (Linux)

Success

Full output at http://107.21.233.14:8877/961fde53b838aef/output.txt

Total script time: 2.20 mins

Published

timvandermeij · 2016-12-16T22:08:23Z

/botio test

pdfjsbot · 2016-12-16T22:08:25Z

From: Bot.io (Linux)

Received

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

Live output at: http://107.21.233.14:8877/a1d9b662179c431/output.txt

pdfjsbot · 2016-12-16T22:08:25Z

From: Bot.io (Windows)

Received

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

Live output at: http://107.22.172.223:8877/d08f4fd63603922/output.txt

pdfjsbot · 2016-12-16T22:34:16Z

From: Bot.io (Windows)

Failed

Full output at http://107.22.172.223:8877/d08f4fd63603922/output.txt

Total script time: 25.85 mins

Font tests: Passed
Unit tests: Passed
Regression tests: FAILED

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

pdfjsbot · 2016-12-16T22:34:50Z

From: Bot.io (Linux)

Failed

Full output at http://107.21.233.14:8877/a1d9b662179c431/output.txt

Total script time: 26.41 mins

Font tests: Passed
Unit tests: Passed
Regression tests: FAILED

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

timvandermeij · 2016-12-16T23:15:40Z

Do you mind reviewing this PR again @Snuffleupagus, in particular the last commit, to check if it's better this way?

Snuffleupagus

Since Yury has already reviewed most of this, I've mainly skimmed through the total diff.

[...] in particular the last commit, to check if it's better this way?

Yes, I think it's much better this way! By directly using the proper elements, the code becomes a lot cleaner and easier to follow!

I've got just one remaining question, and please keep in mind that "no" is a perfectly valid answer:
I've not looked too closely at the individual commits (besides the last one). but would it be possible/useful to fold some the smaller commits together before landing?

Also, when landing this, don't forget to add a note in the tracking issue about the flags that this PR doesn't implement; i.e. NoToggleToOff and RadiosInUnison, see http://www.adobe.com/content/dam/Adobe/en/devnet/acrobat/pdfs/PDF32000_2008.pdf#G11.2201104.

Moreover, ensure that the read-only state is respected and improve CSS names.

…rding to the specification

…ve labels Modern browsers support styling radio buttons and checkboxes with CSS. This makes the implementation much easier, and the fallback for older browsers is still decent.

timvandermeij · 2016-12-17T19:53:23Z

Thank you. I managed to reduce the commits I added from five to three.

timvandermeij · 2016-12-17T19:53:30Z

/botio makeref

pdfjsbot · 2016-12-17T19:53:31Z

From: Bot.io (Windows)

Received

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

Live output at: http://107.22.172.223:8877/59636db25c5db62/output.txt

pdfjsbot · 2016-12-17T19:53:31Z

From: Bot.io (Linux)

Received

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

Live output at: http://107.21.233.14:8877/eb3ae5292f21648/output.txt

pdfjsbot · 2016-12-17T20:18:56Z

From: Bot.io (Linux)

Success

Full output at http://107.21.233.14:8877/eb3ae5292f21648/output.txt

Total script time: 25.42 mins

Lint: Passed
Make references: Passed
Check references: Passed

pdfjsbot · 2016-12-17T20:18:59Z

From: Bot.io (Windows)

Success

Full output at http://107.22.172.223:8877/59636db25c5db62/output.txt

Total script time: 25.46 mins

Lint: Passed
Make references: Passed
Check references: Passed

timvandermeij · 2016-12-17T20:24:11Z

I have updated the tracking issue with the remaining points.

…-radiobutton Interactive forms: render button widget annotations (checkboxes and radio buttons)

timvandermeij added annotations form-acroform labels Dec 15, 2016

timvandermeij mentioned this pull request Dec 15, 2016

Acroform checkbox and radio controls #7664

Closed

yurydelendik approved these changes Dec 16, 2016

View reviewed changes

yurydelendik reviewed Dec 16, 2016

View reviewed changes

timvandermeij force-pushed the acroforms-checkbox-radiobutton branch from 418123c to e3d7ac3 Compare December 16, 2016 00:28

Snuffleupagus reviewed Dec 16, 2016

View reviewed changes

timvandermeij force-pushed the acroforms-checkbox-radiobutton branch 6 times, most recently from 66b0baa to bc5eac0 Compare December 16, 2016 21:52

timvandermeij requested a review from Snuffleupagus December 16, 2016 23:09

Snuffleupagus approved these changes Dec 17, 2016

View reviewed changes

benweet and others added 2 commits December 17, 2016 20:31

Button widget annotations: implement checkboxes and radio buttons

ba012c7

Button widget annotations: implement reference testing

0c9a06c

Moreover, ensure that the read-only state is respected and improve CSS names.

timvandermeij force-pushed the acroforms-checkbox-radiobutton branch from bc5eac0 to 050dc6c Compare December 17, 2016 19:34

Button widget annotations: implement radio button value fetching acco…

77148c7

…rding to the specification

timvandermeij force-pushed the acroforms-checkbox-radiobutton branch from 050dc6c to d5412e1 Compare December 17, 2016 19:37

Button widget annotations: improve unit tests, simplify code and remo…

a428899

…ve labels Modern browsers support styling radio buttons and checkboxes with CSS. This makes the implementation much easier, and the fallback for older browsers is still decent.

timvandermeij force-pushed the acroforms-checkbox-radiobutton branch from d5412e1 to a428899 Compare December 17, 2016 19:42

timvandermeij mentioned this pull request Dec 17, 2016

Interactive form (AcroForm) support #7613

Closed

23 tasks

timvandermeij merged commit 017e9b9 into mozilla:master Dec 17, 2016

timvandermeij deleted the acroforms-checkbox-radiobutton branch December 17, 2016 20:22

timvandermeij mentioned this pull request Dec 17, 2016

fieldValue always empty #6995

Closed

timvandermeij mentioned this pull request Dec 27, 2016

Hyperlinks not extracted for particular PDF #7538

Closed

yurydelendik mentioned this pull request Dec 30, 2016

Unknown property ‘appearance’ #7923

Closed

Snuffleupagus mentioned this pull request Jan 28, 2017

Move EOF/isEOF from core/parser.js to core/primitives.js #8003

Merged

movsb pushed a commit to movsb/pdf.js that referenced this pull request Jul 14, 2018

Merge pull request mozilla#7898 from timvandermeij/acroforms-checkbox…

26aaaea

…-radiobutton Interactive forms: render button widget annotations (checkboxes and radio buttons)

mpw-wwu mentioned this pull request Mar 20, 2019

[AcroTex][LaTeX][eforms] radioButtons not visible #10656

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Interactive forms: render button widget annotations (checkboxes and radio buttons) #7898

Interactive forms: render button widget annotations (checkboxes and radio buttons) #7898

timvandermeij commented Dec 15, 2016 •

edited

Loading

yurydelendik left a comment

yurydelendik Dec 15, 2016

timvandermeij Dec 16, 2016

yurydelendik Dec 16, 2016

timvandermeij Dec 16, 2016

Snuffleupagus left a comment •

edited

Loading

Snuffleupagus Dec 16, 2016 •

edited

Loading

timvandermeij Dec 16, 2016

Snuffleupagus Dec 16, 2016 •

edited

Loading

timvandermeij Dec 16, 2016

timvandermeij commented Dec 16, 2016

pdfjsbot commented Dec 16, 2016

pdfjsbot commented Dec 16, 2016

timvandermeij commented Dec 16, 2016

pdfjsbot commented Dec 16, 2016

pdfjsbot commented Dec 16, 2016

pdfjsbot commented Dec 16, 2016

pdfjsbot commented Dec 16, 2016

timvandermeij commented Dec 16, 2016

Snuffleupagus left a comment •

edited

Loading

timvandermeij commented Dec 17, 2016

timvandermeij commented Dec 17, 2016

pdfjsbot commented Dec 17, 2016

pdfjsbot commented Dec 17, 2016

pdfjsbot commented Dec 17, 2016

pdfjsbot commented Dec 17, 2016

timvandermeij commented Dec 17, 2016

Interactive forms: render button widget annotations (checkboxes and radio buttons) #7898

Interactive forms: render button widget annotations (checkboxes and radio buttons) #7898

Conversation

timvandermeij commented Dec 15, 2016 • edited Loading

yurydelendik left a comment

Choose a reason for hiding this comment

yurydelendik Dec 15, 2016

Choose a reason for hiding this comment

timvandermeij Dec 16, 2016

Choose a reason for hiding this comment

yurydelendik Dec 16, 2016

Choose a reason for hiding this comment

timvandermeij Dec 16, 2016

Choose a reason for hiding this comment

Snuffleupagus left a comment • edited Loading

Choose a reason for hiding this comment

Snuffleupagus Dec 16, 2016 • edited Loading

Choose a reason for hiding this comment

timvandermeij Dec 16, 2016

Choose a reason for hiding this comment

Snuffleupagus Dec 16, 2016 • edited Loading

Choose a reason for hiding this comment

timvandermeij Dec 16, 2016

Choose a reason for hiding this comment

timvandermeij commented Dec 16, 2016

pdfjsbot commented Dec 16, 2016

From: Bot.io (Linux)

Received

pdfjsbot commented Dec 16, 2016

From: Bot.io (Linux)

Success

Published

timvandermeij commented Dec 16, 2016

pdfjsbot commented Dec 16, 2016

From: Bot.io (Linux)

Received

pdfjsbot commented Dec 16, 2016

From: Bot.io (Windows)

Received

pdfjsbot commented Dec 16, 2016

From: Bot.io (Windows)

Failed

pdfjsbot commented Dec 16, 2016

From: Bot.io (Linux)

Failed

timvandermeij commented Dec 16, 2016

Snuffleupagus left a comment • edited Loading

Choose a reason for hiding this comment

timvandermeij commented Dec 17, 2016

timvandermeij commented Dec 17, 2016

pdfjsbot commented Dec 17, 2016

From: Bot.io (Windows)

Received

pdfjsbot commented Dec 17, 2016

From: Bot.io (Linux)

Received

pdfjsbot commented Dec 17, 2016

From: Bot.io (Linux)

Success

pdfjsbot commented Dec 17, 2016

From: Bot.io (Windows)

Success

timvandermeij commented Dec 17, 2016

timvandermeij commented Dec 15, 2016 •

edited

Loading

Snuffleupagus left a comment •

edited

Loading

Snuffleupagus Dec 16, 2016 •

edited

Loading

Snuffleupagus Dec 16, 2016 •

edited

Loading

Snuffleupagus left a comment •

edited

Loading