[poppler] Patches for form choice fields

Thu Nov 1 16:39:30 PDT 2012

On Sunday, October 14, 2012 04:26:02 PM Fabio D'Urso wrote:
> [snip]
> So we have two working options:
>  - Always write /I, not matter if multiselect is true or false (current
>    approach in my 0002 patch)
>  - Only write /I if multiselect is true, but write export values instead of
>    option names in /V, which is also against the specs

Hi,

I'm attaching a new set of patches implementing the second option.
They no longer abuse the /I property for single-selection only fields, instead 
they only break the spec in two things:
 1. They write/expect the export value instead of the option name, if an
    export value is present (see patches 0004 and 0005);
 2. If /I is present, it is given precedence over /V to determine initially
    selected options. In other words, if /I and /V don't agree, /I is used.
    This is the reverse of what the spec says (table 231 - last paragraph),
    but acroread seems to do the same (see patches 0003 and 0006).

Note that #2 only affects how poppler decides which option are initially 
selected. #1 is a little more sensible because it causes poppler to produce 
invalid documents with reference to the PDF spec but, as I've explained in the 
previous posts, tests show that acroread expects *this* behavior and there's 
at least foxit already doing this. Therefore, I feel that we're on a safe 
track.

Details for each patch:

== 0001-FormFieldChoice-ctor-Don-t-convert-human-readable-op.patch ==

With this patch, poppler no longer converts option names to Unicode while 
reading them. Note that frontend clients won't notice the difference because 
both glib and qt4 re-handle encoding conversion internally.

I've removed the conversion so that when we write /V, we set it to a string 
that is binary-equal to the corresponding /Opt entry.
Without this patch, it often occurred that poppler couldn't parse the selected 
option in forms saved by poppler itself: it wrote a Unicode string in /V, but 
when it read the resulting file it compared /V against the strings in /Opt, 
which are usually in pdfdoc encoding. Since it is a binary comparison, all 
tests failed and no selected option was shown.
I could have fixed it by making the comparison encoding-aware, but this 
approach seems to work just fine and therefore I've decided to keep it simple.
Note: Documents saved with old poppler versions will not work, but I haven't 
worried about them as they have never really worked anyway.

== 0002-FormFieldChoice-updateSelection-Fixed-wrong-loop-con.patch ==

Fixes a wrong loop condition. I've put it in a dedicated patch so that you can 
push it to 0.20 too.

== 0003-FormFieldChoice-updateSelection-Write-I-too.patch ==

This patch writes the /I property if the field supports multiple selection or 
removes /I if it doesn't. In other words, this patch makes sure that /I, if 
present, is updated to the current selection. This fixes the situation in 
which the orginal document contained /I, we changed selection (by updating /V 
only) and acroread still showed the old value because /I had not been updated.

Note: This *acroread* behavior is against the spec, that say that "if the 
items identified by I differ from those in the V entry of the field 
dictionary, the V entry shall be used" (table 231 - last paragraph). Note that 
in patch 0006 I knowingly introduce the same bug in poppler too. See below for 
details.

== 0004-FormFieldChoice-ctor-Be-able-to-understand-V-values-.patch,
   0005-FormFieldChoice-updateSelection-Write-the-export-val.patch ===

These two patches break the spec to overcome the issue described here: 
http://lists.freedesktop.org/archives/poppler/2012-October/009641.html .

They make poppler expect/write the export name instead of the option name if 
both are present (i.e. if /Opt is an array of string pairs).

NOTE: With the 5 patches so far, choice fields should be already fully 
working, with the exception of those with duplicate options.

== 0006-FormFieldChoice-ctor-Look-for-selected-options-in-I-.patch ==

This patch adds support to parse selected options from /I.
According to the spec, data in /I should be compared with data in /V and /V 
should be used if they differ. However, acroread seems to blindly trust /I and 
so I've decided to keep things simple and do the same.

Using /I, if available, instead of /V gives us better handling of duplicate 
options (which couldn't be distinguished otherwise) and independence on the 
option name vs export value issue (because we don't look at /V at all).
Note: This patch only adds about 10 lines, the rest is just indentation change

P.S.: I'm working on three more patches about editable combobox fields that 
fix the issue described here:
http://lists.freedesktop.org/archives/poppler/2012-October/009688.html .
I'll post them in that thread.

Thank you,
Fabio
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-FormFieldChoice-ctor-Don-t-convert-human-readable-op.patch
Type: text/x-patch
Size: 2365 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/poppler/attachments/20121102/9e0d68f5/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-FormFieldChoice-updateSelection-Fixed-wrong-loop-con.patch
Type: text/x-patch
Size: 850 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/poppler/attachments/20121102/9e0d68f5/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-FormFieldChoice-updateSelection-Write-I-too.patch
Type: text/x-patch
Size: 2945 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/poppler/attachments/20121102/9e0d68f5/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-FormFieldChoice-ctor-Be-able-to-understand-V-values-.patch
Type: text/x-patch
Size: 2811 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/poppler/attachments/20121102/9e0d68f5/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-FormFieldChoice-updateSelection-Write-the-export-val.patch
Type: text/x-patch
Size: 1403 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/poppler/attachments/20121102/9e0d68f5/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-FormFieldChoice-ctor-Look-for-selected-options-in-I-.patch
Type: text/x-patch
Size: 4051 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/poppler/attachments/20121102/9e0d68f5/attachment-0005.bin>