Basic AcroForms input controls rendering #6172

xlc · 2015-07-03T04:31:02Z

There is already an example implementation of AcroForms in the examples folder. This PR aim to do it properly.

This PR only provide basic implementation of text input fields. Other types of fields (buttons, choice) will be implemented in later PRs.

Issues & Questions:

Printing: Currently only buttons are rendered in canvas and other form elements are not printed.
- We need to either:
  - Render the DOMs before printing?
  - Or render all of them in canvas?
Appearance: Need a way to process appearances and apply them to the DOMs.
How to handle user input values?
- One way is not expose any JS API. If you need to listen for value change, just do $('.annotationLayer .widgetControl').on('change', changeHandler);
- Or add some method on PDFPageView to read/write/listen values.

xlc · 2015-07-06T06:15:09Z

Can anyone point me out how to parse/evaluate "a sequence of valid page-content graphics or text state operators that define such properties as the field’s text size and color" ?
e.g. /TimesNewRomanPSMT 12 Tf 0 0 1 rg
It is the default appearance string of annotations. I can't find any way to use existing code to process it.

timvandermeij · 2015-07-06T08:29:59Z

Let's keep things simple and avoid parsing appearance streams for now. The reason I say this is that this PR has a greater chance of being merged if it is smaller and therefore easier to review and test. We need to split the work here into several PRs and iterate on the issue. For instance, let this PR just be the basic AcroForms framework with basic fields, then a next PR for the other fields, and a next PR for annotation appearance streams. The latter is also affecting my work on the annotation layer, so we need to find out the best way to implement that according to the specification.

xlc · 2015-07-07T04:07:11Z

@timvandermeij Thanks for your suggestion. Can you review the changes?

fbender · 2015-11-16T12:33:01Z

@xlc would you mind splitting up this PR into smaller ones?
@timvandermeij How far along have you come with the annotation layer? What changes have already been merged that obsolete (part of) this PR?

Also see kind-off tracking bug #1459

xlc · 2015-11-16T21:16:48Z

@fbender How should I splitting this? This PR only implements text fields. I have more controls implemented in my other branch which depends on this. I will PR it when this is merged in.

timvandermeij · 2015-11-16T21:53:49Z

Fortunately most of my PR is already merged using smaller PRs (see #5218 (comment) for a complete explanation) and I hope to do the rest soon such that that PR is done. It should not block this PR too much, but we need to have more structure first before adding new annotation features (AcroForms from here, but also the video annotation PR). Part of my PR is the handling of flags in the base class such that subclasses do not need to implement that too (as I see some flag handling in this PR). I also feel that the elements processed here should be in a class that extends the Annotation base class as AcroForm elements are Widgets if I'm not mistaken. We should unify the handling of those somewhat better here.

fbender · 2015-11-17T18:27:41Z

How should I splitting this?

See e.g. @timvandermeij's answer:

For instance, let this PR just be the basic AcroForms framework with basic fields, then a next PR for the other fields, and a next PR for annotation appearance streams.

This needs to be adapted to the new stuff implemented in #5218 first, of course. See e.g.

but we need to have more structure first before adding new annotation features […]. Part of my PR is the handling of flags in the base class such that subclasses do not need to implement that too […]. I also feel that the elements processed here should be in a class that extends the Annotation base class as AcroForm elements are Widgets if I'm not mistaken. We should unify the handling of those somewhat better here.

This somehow seems to contradict this statement:

It should not block this PR too much

@timvandermeij can you please clarify which changes you expect to be blocking this work? Would be nice if there are (merged or still unmerged) PRs (or implementation concepts/drafts) you can link to that will help @xlc in refactoring his code.

timvandermeij · 2015-11-17T22:22:31Z

I meant to say that there are no real dependencies on #5218 for this PR, but that this PR does need a bit more structure and clean-up. Examples:

It needs to be rebased.
It needs unit tests.
The preference name should have "disable" in it instead of "enable" to be in line with all other preferences.
The function getInteractiveHtmlElement seems unnecessary as it is kind of duplicating logic in getHtmlElement.
We should split up the Widget annotation handling into text widgets and form widgets. They are all Widget annotations, but serve a very different purpose. We already have getHtmlElementForTextWidgetAnnotation for regular text widgets, but createTextWidgetAnnotation works on form widgets although the name does not imply that, which is very confusing to me.

I think the bottom line is that I'm not completely sure on what the best structure is. In the PDF specification, Adobe tends to label everything that does not belong to the other annotation types a Widget, but there are many kinds of Widgets (static text widgets, form widgets like buttons and fields, et cetera). I don't think intertwining them all in the code is a good idea: there has to be a clear distinction between form widgets and text widgets, for instance. Ideally every type of annotation, but also every subtype like FormWidgets or TextWidgets, are separate classes. Other ideas for a good structure are of course welcome.

xlc · 2015-11-20T02:54:25Z

AFAIK, Widget annotations are form widgets. There is no static text widget (there is text annotation and free text annotation). Text Field widget is the interactive form widget you can enter text. So I don't think we need distinction between form widgets and text widgets. (Are there any non-form widgets? I can't find form PDF32000 spec)

getHtmlElementForTextWidgetAnnotation is used to display non-interactive text widgets and
createTextWidgetAnnotation is used for interactive text widgets. Both functions works with same widget annotation type but just for different modes.

I can do rebase, refactor code, unit tests, rename preference without too much problem, but I agree we need a good structure first.
Currently in order to implement a new form widget (I have already implemented a few in different branch), I have to modify annotation.js to add a new class subclass from WidgetAnnotation, read whatever fields and parse them and store fields into data object and then modify annotation_helpers.js to implement the logic to render the widget with HTML. Ideally the data object should provide a way to expose all required fields so the logic about a particular widget is in a single place. More importantly, I don't want to modify core pdfjs code in order to implement a feature in viewer. So that other viewer implementations can implement new form widgets or new annotation types without modifying the core pdfs code. (I am basically asking for #5283)

timvandermeij · 2015-11-20T11:11:06Z

@xlc I agree with you that we need to think more about how to structure this properly. Fixing up this PR should not be too hard, but it's the structure that is really important in order to extend this also to all other annotation types.

timvandermeij · 2015-12-15T14:59:37Z

src/core/annotation.js

@@ -634,7 +635,7 @@ var LinkAnnotation = (function LinkAnnotationClosure() {
        if (!isValidUrl(url, false)) {
          url = '';
        }
-        // According to ISO 32000-1:2008, section 12.6.4.7, 
+        // According to ISO 32000-1:2008, section 12.6.4.7,


This change is no longer necessary when you rebase the patch as it has been fixed already in another commit.

timvandermeij · 2016-02-13T19:14:27Z

This PR needs a rebase now that your other PR landed.

timvandermeij · 2016-02-13T19:17:01Z

src/display/annotation_layer.js

+      if (!isMultiline && !('fontSize' in this.data)) {
+        // hack to guess font size base on content hight
+        // so it display fine on small text field
+        // this need to be removed when we can apply defaultAppearance


Change this to // TODO: remove this when we can apply the default appearance.

xlc · 2016-02-14T22:40:48Z

@timvandermeij Updated.

timvandermeij · 2016-02-15T12:02:14Z

src/display/annotation_layer.js

-      this.container.appendChild(content);
-      return this.container;
+      if (!isMultiline && !('fontSize' in this.data)) {
+        // hack to guess font size based on content hight


Nit: start with a capital H and change "hight" to "height"

timvandermeij · 2016-02-15T12:02:49Z

This is looking much better than before. Could you address the other nit above and then squash the commits into one?

Snuffleupagus · 2016-02-15T12:04:29Z

Also, unless I'm overlooking something, isn't #6172 (comment) still unaddressed?

timvandermeij · 2016-02-15T12:06:32Z

Good point, that one has to be addressed too.

xlc · 2016-02-15T22:12:25Z

What will be the best way to save the values? Should I create a FormFieldStorage class to store the values? Or add a field to other existing class?

xlc · 2016-02-16T22:56:32Z

src/core/annotation.js

+          include = false;
+        }
+      }
+      if (include) {


Testing file: https://drive.google.com/file/d/0B6YhVF9p-y2AbS1wY3drMDdGRVE/view?usp=sharing

xlc · 2016-02-16T22:57:40Z

@Snuffleupagus @timvandermeij I have implemented a storage solution, what do you think?

xlc · 2016-03-10T22:16:34Z

@timvandermeij Can you have a look?

fbender · 2016-05-20T19:01:36Z

@xlc The branch needs to be rebased. Care to do that and ping @Snuffleupagus again afterwards?

timvandermeij · 2016-09-17T18:34:17Z

Closing as superseded by a series of merged PRs that implemented this functionality. The remaining tasks (printing, storage and appearances) are tracked in #7613.

timvandermeij added viewer form-acroform labels Jul 3, 2015

xlc force-pushed the forms branch 2 times, most recently from ba370ae to 090536e Compare July 6, 2015 06:09

xlc force-pushed the forms branch 4 times, most recently from e588320 to 92bee91 Compare July 7, 2015 04:04

xlc force-pushed the forms branch 6 times, most recently from 481331d to 24228ce Compare July 9, 2015 03:00

yurydelendik mentioned this pull request Nov 11, 2015

radio buttons should have same field names #6623

Closed

timvandermeij reviewed Dec 15, 2015
View reviewed changes

timvandermeij added the annotations label Dec 15, 2015

timvandermeij reviewed Feb 13, 2016
View reviewed changes

xlc force-pushed the forms branch 2 times, most recently from dc20ff9 to 78cf950 Compare February 14, 2016 22:04

timvandermeij reviewed Feb 15, 2016
View reviewed changes

xlc force-pushed the forms branch 2 times, most recently from 54e9f5d to b6c9c04 Compare February 15, 2016 22:07

xlc reviewed Feb 16, 2016
View reviewed changes

xlc force-pushed the forms branch from 72eebcf to 701c99c Compare March 10, 2016 22:15

xlc added 3 commits March 11, 2016 11:21

implement interactive text forms

a40d59d

avoid render form field twice

4b2eeb8

add storage service and use it to store form field value

e67e316

xlc force-pushed the forms branch from 701c99c to e67e316 Compare March 10, 2016 22:21

timvandermeij mentioned this pull request Jun 15, 2016

Add support for ResetForm Action #7412

Closed

timvandermeij closed this Sep 17, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Basic AcroForms input controls rendering #6172

Basic AcroForms input controls rendering #6172

xlc commented Jul 3, 2015

xlc commented Jul 6, 2015

timvandermeij commented Jul 6, 2015

xlc commented Jul 7, 2015

fbender commented Nov 16, 2015

xlc commented Nov 16, 2015

timvandermeij commented Nov 16, 2015

fbender commented Nov 17, 2015

timvandermeij commented Nov 17, 2015

xlc commented Nov 20, 2015

timvandermeij commented Nov 20, 2015

timvandermeij Dec 15, 2015

timvandermeij commented Feb 13, 2016

timvandermeij Feb 13, 2016

xlc commented Feb 14, 2016

timvandermeij Feb 15, 2016

timvandermeij commented Feb 15, 2016

Snuffleupagus commented Feb 15, 2016

timvandermeij commented Feb 15, 2016

xlc commented Feb 15, 2016

xlc Feb 16, 2016

xlc commented Feb 16, 2016

xlc commented Mar 10, 2016

fbender commented May 20, 2016

timvandermeij commented Sep 17, 2016 •

edited

Loading

Basic AcroForms input controls rendering #6172

Basic AcroForms input controls rendering #6172

Conversation

xlc commented Jul 3, 2015

xlc commented Jul 6, 2015

timvandermeij commented Jul 6, 2015

xlc commented Jul 7, 2015

fbender commented Nov 16, 2015

xlc commented Nov 16, 2015

timvandermeij commented Nov 16, 2015

fbender commented Nov 17, 2015

timvandermeij commented Nov 17, 2015

xlc commented Nov 20, 2015

timvandermeij commented Nov 20, 2015

timvandermeij Dec 15, 2015

Choose a reason for hiding this comment

timvandermeij commented Feb 13, 2016

timvandermeij Feb 13, 2016

Choose a reason for hiding this comment

xlc commented Feb 14, 2016

timvandermeij Feb 15, 2016

Choose a reason for hiding this comment

timvandermeij commented Feb 15, 2016

Snuffleupagus commented Feb 15, 2016

timvandermeij commented Feb 15, 2016

xlc commented Feb 15, 2016

xlc Feb 16, 2016

Choose a reason for hiding this comment

xlc commented Feb 16, 2016

xlc commented Mar 10, 2016

fbender commented May 20, 2016

timvandermeij commented Sep 17, 2016 • edited Loading

timvandermeij commented Sep 17, 2016 •

edited

Loading