[poppler] Patches for form choice fields

Sat Nov 3 02:07:19 PDT 2012

Fabio D'Urso <fabiodurso at hotmail.it> writes:

> 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,

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.

Pushed to the stable branch 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.

I've pushed these merged.

> == 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

Editable form field choices are currently broken in evince, but I've
tried the patches with poppler-glib-demo and they work as expected
(matching acroread behaviour).

> 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.

Thanks!

> Thank you,
> Fabio

-- 
Carlos Garcia Campos
PGP key: http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x523E6462
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/poppler/attachments/20121103/e0fbe413/attachment-0001.pgp>