[poppler] Patches for form choice fields

[Carlos Garcia Campos]

Excerpts from Fabio D'Urso's message of dom oct 14 03:21:18 +0200 2012:
> Hi,
> 
> the attached patches fix some issues I've found testing form code on the 
> attached testcase (http://www.pureforms.com/Products/DEMO/WebDemo/WebToDB.pdf)
> 
> With the patches from the other mail (thread "Filled text field data not 
> visible in saved document"), non-choice fields already work properly and 
> their contents can be read from acroread. Patches in this message must be 
> applied on top of patch "AnnotWidget: Avoid repeatedly [..]" from that thread.
> 
> These patches are focused on the two choice fields at the bottom of 
> the first page.
> 
> 0001
> 
> The first patch fixes a wrong index variable used to scan the list of the 
> elements that are initially selected (/V property) in case of multiple 
> selection. This issue makes Poppler crash.
> Steps to reproduce: Open the attached document, select two items in the choice 
> field on the right (eg 'Item 3' and 'Item 4'), save to PDF. Poppler crashes 
> when opening the resulting PDF document.

Pushed!

> 0002
> 
> This patch makes it possible to see what item(s) is/are selected when you open 
> the resulting PDF in acroread. Without this patch, acroread always displays 
> "Blank equivalent" in the field on the left, and initially always selects 
> "Blank equivalent" if you try to edit the field on the right.
> This patch adds support to write the /I field, which is the list of the 
> indices of the selected items. According to PDF specs, it is mandatory only 
> under some circumstances, but tests showed that acroread always require it.

Even for non multiselect form choice fields? The spec says "This entry
should not be used for choice fields that do not allow multiple
selection", so I think we should not write the I entry when
isMultiSelect is false.

> - How to reproduce the issue in the field on the left:
> Steps: Open the attached document, select "Mars", save. Acroread still shows 
> "Blank equivalent" instead of "Mars".
> With the patch: Acroread now shows "Mars" and the entry is highlighted in the 
> dropdown list.
> 
> - How to reproduce the issue in the field on the right:
> Steps: Open the attached document, select "Item 3", save. Acroread correctly 
> highlights "Item 3" but if you try to add "Item 1" (CTRL+click) then you see 
> "Item 3" deselected and "Blank equivalent" selected instead. Actually, 
> acroread has never thought of Item 3 being selected, it just showed it black 
> because the appearance stream generated by Poppler (thanks to patches from the 
> other thread) told it so.
> With the patch: The same sequence of steps results in both "Item 1" and "Item 
> 3" being selected in acroread.
>
> 0003
> 
> Refactoring of code that converts optionName strings to UTF16 in 
> FormFieldChoice. Needed for the next patch.
> 
> Note that optionName strings were not converted to UTF16 if /V was neither an 
> String nor an Array. Since in a valid document /V is always either String or 
> Array, I assumed it was an error in the original code. Now the conversion is 
> always done, regardless of the /V object type.
> 
> 0004
> 
> This last patch adds support to read the /I field, so that we can properly 
> read back forms filled by Poppler itself. Without this patch, Poppler only 
> *shows* selected values (because that's what the appearance stream says), but 
> doesn't detect them.
> Steps to reproduce (in Okular): Select something, save, open the resulting 
> document in Okular, activate edit mode, notice that the selection is lost.
> 
> Actually, we already have code to parse /V and find out what items are 
> selected, but it doesn't work properly with documents filled by Poppler 
> itself because we write UTF16-converted strings in /V but, when opening a 
> document, we compare /V against entries in /Opt, that are in PdfDocEncoding. 
> Thus, these tests always fail.

I think we should fix this anyway for documents containing single
selection choice fields, or previously saved by poppler. 

> I could have changed this piece of code to make it aware of different 
> encodings but it seemed to me that reading /I is a more robust option for two 
> reasons:
>  - it better deals with duplicate items (ie items with the same value)
>  - foxit reader stores the wrong value in /V: it writes what we call
>    "exportVal" instead of what we expect ("optionName"). The /I field is
>    always correct instead.

I guess we shouldn't decide our code based on foxit bugs.

> If /I is not available, the patch falls back to existing code that parses /V.
> 
> Note for reviewers: In patch 0004 I've only added a few lines, the rest is 
> just indentation change.
> 
> In general choice fields are expected to be fully working now, with the 
> exception of editable combobox fields, ie those where you can either select a 
> predefined value or type your own. At least using okular, custom values don't 
> work yet. I'll have a look at this issue tomorrow.
> 
> Please let me know if you find any other issue.

Great work, thanks. 

> Thank you,
> Fabio
-- 
Carlos Garcia Campos
PGP key: http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x523E6462