[
https://issues.apache.org/jira/browse/PDFBOX-2580?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14310925#comment-14310925
]
John Hewson edited comment on PDFBOX-2580 at 2/7/15 8:54 PM:
-------------------------------------------------------------
Thanks for the detailed explanation. The changes which you're proposing to the
forms API are pretty big and the specifics need to be proposed in more detail
before we have something which everyone is happy with.
{quote}
The next step - and this is one of the reasons why the .services.forms package
has been created - is to further enhance the usability of forms filling and
creation. As of today creating a new form field users need to know that they
have to create a field, have to create an annotation, set the annotation to be
used within the field, apply styling to the annotation .... - not very user
friendly.
{quote}
That's a very good goal but the place to add this functionality is in the PD
model, as that where our high-level code lives., e.g. the new font embedding
features are very sophisticated and are entirely implemented within the PD
model. Convenience methods for adding form fields can be added to PDPage,
perhaps by replacing List<PDAnnotation> returned by getAnnotations() with a
PDAnnotations (note the 's') object which implements List<PDAnnotation> but
also provides all the necessary helper methods for creating new annotations,
including form fields. Alternatively there may be other designs such as having
a PDFormFields getFormFields() method on PDDocument, where PDFormFields
performs the high-level convince functions - whatever is the most appropriate
design. Can you suggest some appropriate PD classes along these lines? Maybe we
can figure out the class structure right now.
{quote}
Further more appearance generation can't be made dependent on setting the field
value and only done there as there are forms where the appearance needs to be
regenerated using the current field value. This is the case if the
NeedAppearances flag has been set in the form. E.g. if a form has that flag set
and we do render the form, prior to doing the rendering the appearance stream
for the AcroForm fields needs to be (re-) generated. That is the reason why the
AppearanceGenerator is publicly available and not package private and not
hidden behind the field value setting.
{quote}
This is one of those low-level details which we need to figure out early on,
because everything else is going to depend upon it. I agree that we need
appearance generation to occur in situations other than when the field value is
set, such as when rendering a field which has NeedAppearances. However, there's
no reason why that code can't have PDField as its public-facing API and be
package-private in pdmodel.interactive.form, e.g. if we want an appearance for
a field when rendering, then we should add a getAppearance() method to PDField,
which returns a generated content stream. By making the helper class public, we
expose an internal implementation detail of the pdmodel.interactive.form
package, which increases cross-package coupling (because PDForm still depends
on that helper class) and means that those details become part of our public
API and hence difficult to change - Sonar has spotted this problem in your Dec
22 commits, I've attached a screenshot in [^sonar.png].
{quote}
So .services.forms should be the new home for a higher level forms API (of
course the package name could be discussed upon but I didn't want to make a
package directly under o.a.pdfbox as a similar approach needs to be taken for
annotations). The reason I'm establishing it now is to be able to build on that
developing that higher level API without the need to do another major release
to repackage. So user can expect and trust that a high level AcroForms API is
in that package with further abstractions to our low level PD (or even COS)
model.
{quote}
I think I've covered this in my last few comments already, so I'll just repeat
that PD is our high-level API (at least for single-document manipulation) and
there's nothing specific to forms which requires a higher-level API above COS
and PD. But we do need to increase PD's high-level capability with regards to
forms, by enhancing the PD model as I suggest above.
{quote}
Finally wrt to naming. The name PDAppearanceString has been left intact while
I'm reworking the very procedural code. There is nothing like an
AppearanceString in PDF. The current functionality is something between
handling a Default Appearance String and creating an appearance stream.
{quote}
This is another low-level detail which we need to give some thought, as the
class originally came from the PD model, it's worth asking ourselves what it
really represents, and whether or not it should be replaced with something
which better models how PDF actually works. At a glance it looks like this
class is, as you say not really a first-class PD concept, however what it does
look like is simply a helper class for PDField, which is why I say it is an
implementation detail.
was (Author: jahewson):
Thanks for the detailed explanation. The changes which you're proposing to the
forms API are pretty big and the specifics need to be proposed in more detail
before we have something which everyone is happy with.
{quote}
The next step - and this is one of the reasons why the .services.forms package
has been created - is to further enhance the usability of forms filling and
creation. As of today creating a new form field users need to know that they
have to create a field, have to create an annotation, set the annotation to be
used within the field, apply styling to the annotation .... - not very user
friendly.
{quote}
That's a very good goal but the place to add this functionality is in the PD
model, as that where our high-level code lives., e.g. the new font embedding
features are very sophisticated and are entirely implemented within the PD
model. Convenience methods for adding form fields can be added to PDPage,
perhaps by replacing List<PDAnnotation> returned by getAnnotations() with a
PDAnnotations (note the 's') object which implements List<PDAnnotation> but
also provides all the necessary helper methods for creating new annotations,
including form fields. Alternatively there may be other designs such as having
a PDFormFields getFormFields() method on PDDocument, where PDFormFields
performs the high-level convince functions - whatever is the most appropriate
design. Can you suggest some appropriate PD classes along these lines? Maybe we
can figure out the class structure right now.
{quote}
Further more appearance generation can't be made dependent on setting the field
value and only done there as there are forms where the appearance needs to be
regenerated using the current field value. This is the case if the
NeedAppearances flag has been set in the form. E.g. if a form has that flag set
and we do render the form, prior to doing the rendering the appearance stream
for the AcroForm fields needs to be (re-) generated. That is the reason why the
AppearanceGenerator is publicly available and not package private and not
hidden behind the field value setting.
{quote}
This is one of those low-level details which we need to figure out early on,
because everything else is going to depend upon it. I agree that we need
appearance generation to occur in situations other than when the field value is
set, such as when rendering a field which has NeedAppearances. However, there's
no reason why that code can't have PDField as its public-facing API and be
package-private in pdmodel.interactive.form, e.g. if we want an appearance for
a field when rendering, then we should add a getAppearance() method to PDField,
which returns a generated content stream. By making the helper class public, we
expose an internal implementation detail of the pdmodel.interactive.form
package, which increases cross-package coupling (because PDForm still depends
on that helper class) and means that those details become part of our public
API and hence difficult to change - Sonar has spotted this problem in your Dec
22 commits, I've attached a screenshot in [^sonar.png].
{quote}
So .services.forms should be the new home for a higher level forms API (of
course the package name could be discussed upon but I didn't want to make a
package directly under o.a.pdfbox as a similar approach needs to be taken for
annotations). The reason I'm establishing it now is to be able to build on that
developing that higher level API without the need to do another major release
to repackage. So user can expect and trust that a high level AcroForms API is
in that package with further abstractions to our low level PD (or even COS)
model.
{quote}
I think I've covered this in my last few comments already, so I'll just repeat
that PD is our high-level API (at least for single-document manipulation) and
there's nothing specific to forms which requires a higher-level API above COS
and PD. But we do need to increase PD's high-level capability with regards to
forms, by enhancing the PD model as I hint at above.
{quote}
Finally wrt to naming. The name PDAppearanceString has been left intact while
I'm reworking the very procedural code. There is nothing like an
AppearanceString in PDF. The current functionality is something between
handling a Default Appearance String and creating an appearance stream.
{quote}
This is another low-level detail which we need to give some thought, as the
class originally came from the PD model, it's worth asking ourselves what it
really represents, and whether or not it should be replaced with something
which better models how PDF actually works. At a glance it looks like this
class is, as you say not really a first-class PD concept, however what it does
look like is simply a helper class for PDField, which is why I say it is an
implementation detail.
> Decouple implementation specific forms handling from interactive.form PD Model
> ------------------------------------------------------------------------------
>
> Key: PDFBOX-2580
> URL: https://issues.apache.org/jira/browse/PDFBOX-2580
> Project: PDFBox
> Issue Type: Improvement
> Components: AcroForm
> Reporter: Maruan Sahyoun
> Assignee: Maruan Sahyoun
> Fix For: 2.0.0
>
> Attachments: sonar.png
>
>
> The interactive.form PD model currently holds classes reflecting the various
> fields intermixed with appearance generation and layout handling.
> In order to separate the PD model from the service of forms filling and
> appearance generation this functionality shall be moved into a new package.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
