On Sunday, October 14, 2012 11:17:19 AM Carlos Garcia Campos wrote:
> Excerpts from Fabio D'Urso's message of dom oct 14 03:21:18 +0200 2012:
Hi Carlos,
> > 0002
> > [...]
> > 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.
I had tested it, but acroread then failed to recognize the currently selected
item: it correctly shows the value (eg "Green") but doesn't highlight it in
the dropdown box. In other words, I guess that acroread thinks that "Green" is
a value typed by the user.
I'm attaching test1.diff for you to test yourself.
At first I tought that it was that same encoding issue we have (ie we write
UTF16 but acroread expects PdfDocEncoding), so I had also tested test2.diff,
which didn't work either.
Now, I've just made a new test (test3.diff), that writes the export value
instead of the option name. This is the same incorrect behavior (*) as foxit
but, suprisingly, it works.
* = See the last paragraph in 12.7.4.4 "Choice fields"
To sum up, yesterday it seemed to me that acroread always expects /I, even
when the specs say it shouldn't. Actually acroread doesn't, but it incorrectly
expects /V to contain the "export value" instead of the "option name", and
that's why my tests showed that /V was ignored.
If /I is present, acroread gives it precedence over /V [test4.diff] (once
again, against the specs, see last line in table 231).
Quite confusing... :D
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 [alternative0002.patch]
>
> [...]
> > 0004
>
> I think we should fix this anyway for documents containing single
> selection choice fields, or previously saved by poppler.
Sure we can, especially if we decide not to write /I if !multiselect.
On the other hand, I'd not consider old poppler-saved documents a priority
because those documents never worked properly: neither acroread nor poppler
itself could read them back correctly.
> > 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.
I agree in general, in this specific case this "workaround" came at no cost.
Now, given the results of test3.diff and alternative0002.patch, I begin to
doubt it was a foxit bug at all...
Fabio
diff --git a/poppler/Form.cc b/poppler/Form.cc
index 7c30cdc..335b8f7 100644
--- a/poppler/Form.cc
+++ b/poppler/Form.cc
@@ -1237,6 +1237,11 @@ void FormFieldChoice::updateSelection() {
}
}
+ if (!multiselect) {
+ objI.free();
+ objI.initNull(); // a null object removes the entry
+ }
+
obj.getDict()->set("V", &objV);
obj.getDict()->set("I", &objI);
xref->setModifiedObject(&obj, ref);
diff --git a/poppler/Form.cc b/poppler/Form.cc
index 7c30cdc..c44991c 100644
--- a/poppler/Form.cc
+++ b/poppler/Form.cc
@@ -1205,37 +1205,9 @@ void FormFieldChoice::print(int indent)
void FormFieldChoice::updateSelection() {
Object objV, objI;
- //this is an editable combo-box with user-entered text
- if (edit && editedChoice) {
- objV.initString(editedChoice->copy());
- objI.initNull();
- } else {
- int numSelected = getNumSelected();
- if (numSelected == 0) {
- objV.initString(new GooString(""));
- objI.initNull();
- } else if (numSelected == 1) {
- objI.initArray(xref);
- for (int i = 0; numChoices; i++) {
- if (choices[i].optionName && choices[i].selected) {
- Object obj1;
- objV.initString(choices[i].optionName->copy());
- objI.arrayAdd(obj1.initInt(i));
- break;
- }
- }
- } else {
- objV.initArray(xref);
- objI.initArray(xref);
- for (int i = 0; i < numChoices; i++) {
- if (choices[i].optionName && choices[i].selected) {
- Object obj1;
- objV.arrayAdd(obj1.initString(choices[i].optionName->copy()));
- objI.arrayAdd(obj1.initInt(i));
- }
- }
- }
- }
+ // HACK: Hardcoded "Green" selection in PdfDocEncoding, don't write /I
+ objI.initNull(); // a null object removes the entry
+ objV.initString(new GooString("Green"));
obj.getDict()->set("V", &objV);
obj.getDict()->set("I", &objI);
diff --git a/poppler/Form.cc b/poppler/Form.cc
index 7c30cdc..9570313 100644
--- a/poppler/Form.cc
+++ b/poppler/Form.cc
@@ -1205,37 +1205,9 @@ void FormFieldChoice::print(int indent)
void FormFieldChoice::updateSelection() {
Object objV, objI;
- //this is an editable combo-box with user-entered text
- if (edit && editedChoice) {
- objV.initString(editedChoice->copy());
- objI.initNull();
- } else {
- int numSelected = getNumSelected();
- if (numSelected == 0) {
- objV.initString(new GooString(""));
- objI.initNull();
- } else if (numSelected == 1) {
- objI.initArray(xref);
- for (int i = 0; numChoices; i++) {
- if (choices[i].optionName && choices[i].selected) {
- Object obj1;
- objV.initString(choices[i].optionName->copy());
- objI.arrayAdd(obj1.initInt(i));
- break;
- }
- }
- } else {
- objV.initArray(xref);
- objI.initArray(xref);
- for (int i = 0; i < numChoices; i++) {
- if (choices[i].optionName && choices[i].selected) {
- Object obj1;
- objV.arrayAdd(obj1.initString(choices[i].optionName->copy()));
- objI.arrayAdd(obj1.initInt(i));
- }
- }
- }
- }
+ // HACK: Hardcoded "green" selection in PdfDocEncoding, don't write /I
+ objI.initNull(); // a null object removes the entry
+ objV.initString(new GooString("green")); // <-- against the specs: this is the export value, not the option name
obj.getDict()->set("V", &objV);
obj.getDict()->set("I", &objI);
diff --git a/poppler/Form.cc b/poppler/Form.cc
index 7c30cdc..f7185fd 100644
--- a/poppler/Form.cc
+++ b/poppler/Form.cc
@@ -1203,39 +1203,11 @@ void FormFieldChoice::print(int indent)
#endif
void FormFieldChoice::updateSelection() {
- Object objV, objI;
+ Object objV, objI, obj1;
- //this is an editable combo-box with user-entered text
- if (edit && editedChoice) {
- objV.initString(editedChoice->copy());
- objI.initNull();
- } else {
- int numSelected = getNumSelected();
- if (numSelected == 0) {
- objV.initString(new GooString(""));
- objI.initNull();
- } else if (numSelected == 1) {
- objI.initArray(xref);
- for (int i = 0; numChoices; i++) {
- if (choices[i].optionName && choices[i].selected) {
- Object obj1;
- objV.initString(choices[i].optionName->copy());
- objI.arrayAdd(obj1.initInt(i));
- break;
- }
- }
- } else {
- objV.initArray(xref);
- objI.initArray(xref);
- for (int i = 0; i < numChoices; i++) {
- if (choices[i].optionName && choices[i].selected) {
- Object obj1;
- objV.arrayAdd(obj1.initString(choices[i].optionName->copy()));
- objI.arrayAdd(obj1.initInt(i));
- }
- }
- }
- }
+ // This is like test3.diff, but with /I selecting "Mars"
+ objI.initArray(xref)->arrayAdd(obj1.initInt(4));
+ objV.initString(new GooString("green")); // <-- against the specs: this is the export value, not the option name
obj.getDict()->set("V", &objV);
obj.getDict()->set("I", &objI);
From 3d0e5abc5abc3d60b11128413ea52594ef3082f5 Mon Sep 17 00:00:00 2001
From: Fabio D'Urso <[email protected]>
Date: Sat, 13 Oct 2012 00:13:33 +0200
Subject: [PATCH] *Proof of concept* -- Alternative 0002 patch
Only write /I when specs say it should be done.
Write the export value instead of option name <--- This is against the specs
---
poppler/Form.cc | 35 ++++++++++++++++++++++++-----------
1 files changed, 24 insertions(+), 11 deletions(-)
diff --git a/poppler/Form.cc b/poppler/Form.cc
index 68a4219..835e9d2 100644
--- a/poppler/Form.cc
+++ b/poppler/Form.cc
@@ -1193,35 +1193,48 @@ void FormFieldChoice::print(int indent)
#endif
void FormFieldChoice::updateSelection() {
- Object obj1;
+ Object objV, objI;
//this is an editable combo-box with user-entered text
if (edit && editedChoice) {
- obj1.initString(editedChoice->copy());
+ objV.initString(editedChoice->copy());
+ objI.initNull();
} else {
int numSelected = getNumSelected();
if (numSelected == 0) {
- obj1.initString(new GooString(""));
+ objV.initString(new GooString(""));
+ objI.initNull();
} else if (numSelected == 1) {
+ objI.initArray(xref);
for (int i = 0; numChoices; i++) {
- if (choices[i].optionName && choices[i].selected) {
- obj1.initString(choices[i].optionName->copy());
+ if (choices[i].exportVal && choices[i].selected) {
+ Object obj1;
+ objV.initString(choices[i].exportVal->copy());
+ objI.arrayAdd(obj1.initInt(i));
break;
}
}
} else {
- obj1.initArray(xref);
+ objV.initArray(xref);
+ objI.initArray(xref);
for (int i = 0; i < numChoices; i++) {
- if (choices[i].optionName && choices[i].selected) {
- Object obj2;
- obj2.initString(choices[i].optionName->copy());
- obj1.arrayAdd(&obj2);
+ if (choices[i].exportVal && choices[i].selected) {
+ Object obj1;
+ objV.arrayAdd(obj1.initString(choices[i].exportVal->copy()));
+ objI.arrayAdd(obj1.initInt(i));
}
}
}
}
- obj.getDict()->set("V", &obj1);
+ // Do not write /I if multiple selection is not allowed
+ if (!multiselect) {
+ objI.free();
+ objI.initNull(); // a null object removes the entry
+ }
+
+ obj.getDict()->set("V", &objV);
+ obj.getDict()->set("I", &objI);
xref->setModifiedObject(&obj, ref);
updateChildrenAppearance();
}
--
1.7.6.5
_______________________________________________
poppler mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/poppler
