Merge pull request #12263 from timvandermeij/acroform-fixes

Fix AcroForm printing/saving edge cases

src/core/annotation.js

@@ -803,11 +803,14 @@ class WidgetAnnotation extends Annotation {
 
     data.annotationType = AnnotationType.WIDGET;
     data.fieldName = this._constructFieldName(dict);
-    data.fieldValue = getInheritableProperty({
+
+    const fieldValue = getInheritableProperty({
       dict,
       key: "V",
       getArray: true,
     });
+    data.fieldValue = this._decodeFormValue(fieldValue);
+
     data.alternativeText = stringToPDFString(dict.get("TU") || "");
     data.defaultAppearance =
       getInheritableProperty({ dict, key: "DA" }) ||
@@ -882,6 +885,28 @@ class WidgetAnnotation extends Annotation {
     return fieldName.join(".");
   }
 
+  /**
+   * Decode the given form value.
+   *
+   * @private
+   * @memberof WidgetAnnotation
+   * @param {Array<string>|Name|string} formValue - The (possibly encoded)
+   *   form value.
+   * @returns {Array<string>|string|null}
+   */
+  _decodeFormValue(formValue) {
+    if (Array.isArray(formValue)) {
+      return formValue
+        .filter(item => isString(item))
+        .map(item => stringToPDFString(item));
+    } else if (isName(formValue)) {
+      return stringToPDFString(formValue.name);
+    } else if (isString(formValue)) {
+      return stringToPDFString(formValue);
+    }
+    return null;
+  }
+
   /**
    * Check if a provided field flag is set.
    *
@@ -1194,7 +1219,9 @@ class TextWidgetAnnotation extends WidgetAnnotation {
     const dict = params.dict;
 
     // The field value is always a string.
-    this.data.fieldValue = stringToPDFString(this.data.fieldValue || "");
+    if (!isString(this.data.fieldValue)) {
+      this.data.fieldValue = "";
+    }
 
     // Determine the alignment of text in the field.
     let alignment = getInheritableProperty({ dict, key: "Q" });
@@ -1499,34 +1526,28 @@ class ButtonWidgetAnnotation extends WidgetAnnotation {
   }
 
   _processCheckBox(params) {
-    if (isName(this.data.fieldValue)) {
-      this.data.fieldValue = this.data.fieldValue.name;
-    }
-
     const customAppearance = params.dict.get("AP");
     if (!isDict(customAppearance)) {
       return;
     }
 
-    const exportValueOptionsDict = customAppearance.get("D");
+    const normalAppearance = customAppearance.get("N");
-    if (!isDict(exportValueOptionsDict)) {
+    if (!isDict(normalAppearance)) {
       return;
     }
 
-    const exportValues = exportValueOptionsDict.getKeys();
+    const exportValues = normalAppearance.getKeys();
-    const hasCorrectOptionCount = exportValues.length === 2;
+    if (!exportValues.includes("Off")) {
-    if (!hasCorrectOptionCount) {
+      // The /Off appearance is optional.
+      exportValues.push("Off");
+    }
+    if (exportValues.length !== 2) {
       return;
     }
 
     this.data.exportValue =
       exportValues[0] === "Off" ? exportValues[1] : exportValues[0];
 
-    const normalAppearance = customAppearance.get("N");
-    if (!isDict(normalAppearance)) {
-      return;
-    }
-
     this.checkedAppearance = normalAppearance.get(this.data.exportValue);
     this.uncheckedAppearance = normalAppearance.get("Off") || null;
   }
@@ -1541,7 +1562,7 @@ class ButtonWidgetAnnotation extends WidgetAnnotation {
       const fieldParentValue = fieldParent.get("V");
       if (isName(fieldParentValue)) {
         this.parent = params.dict.getRaw("Parent");
-        this.data.fieldValue = fieldParentValue.name;
+        this.data.fieldValue = this._decodeFormValue(fieldParentValue);
       }
     }
 
@@ -1602,8 +1623,10 @@ class ChoiceWidgetAnnotation extends WidgetAnnotation {
         const isOptionArray = Array.isArray(option);
 
         this.data.options[i] = {
-          exportValue: isOptionArray ? xref.fetchIfRef(option[0]) : option,
+          exportValue: this._decodeFormValue(
-          displayValue: stringToPDFString(
+            isOptionArray ? xref.fetchIfRef(option[0]) : option
+          ),
+          displayValue: this._decodeFormValue(
             isOptionArray ? xref.fetchIfRef(option[1]) : option
           ),
         };

src/display/annotation_layer.js

@@ -665,6 +665,18 @@ class ChoiceWidgetAnnotationElement extends WidgetAnnotationElement {
     const storage = this.annotationStorage;
     const id = this.data.id;
 
+    // For printing/saving we currently only support choice widgets with one
+    // option selection. Therefore, listboxes (#12189) and comboboxes (#12224)
+    // are not properly printed/saved yet, so we only store the first item in
+    // the field value array instead of the entire array. Once support for those
+    // two field types is implemented, we should use the same pattern as the
+    // other interactive widgets where the return value of `getOrCreateValue` is
+    // used and the full array of field values is stored.
+    storage.getOrCreateValue(
+      id,
+      this.data.fieldValue.length > 0 ? this.data.fieldValue[0] : null
+    );
+
     const selectElement = document.createElement("select");
     selectElement.disabled = this.data.readOnly;
     selectElement.name = this.data.fieldName;
@@ -682,16 +694,15 @@ class ChoiceWidgetAnnotationElement extends WidgetAnnotationElement {
       const optionElement = document.createElement("option");
       optionElement.textContent = option.displayValue;
       optionElement.value = option.exportValue;
-      if (this.data.fieldValue.includes(option.displayValue)) {
+      if (this.data.fieldValue.includes(option.exportValue)) {
         optionElement.setAttribute("selected", true);
-        storage.setValue(id, option.displayValue);
       }
       selectElement.appendChild(optionElement);
     }
 
     selectElement.addEventListener("input", function (event) {
       const options = event.target.options;
-      const value = options[options.selectedIndex].text;
+      const value = options[options.selectedIndex].value;
       storage.setValue(id, value);
     });
 

test/pdfs/issue12233.pdf.link

@@ -0,0 +1 @@
+https://github.com/mozilla/pdf.js/files/5112498/OoPdfFormExample.pdf

test/test_manifest.json

@@ -4492,6 +4492,25 @@
          "63R": ""
        }
     },
+    {  "id": "issue12233-forms",
+       "file": "pdfs/issue12233.pdf",
+       "md5": "6099fc695fe018ce444752929d86f9c8",
+       "link": true,
+       "rounds": 1,
+       "type": "eq",
+       "forms": true
+    },
+    {  "id": "issue12233-print",
+       "file": "pdfs/issue12233.pdf",
+       "md5": "6099fc695fe018ce444752929d86f9c8",
+       "link": true,
+       "rounds": 1,
+       "type": "eq",
+       "print": true,
+       "annotationStorage": {
+         "20R": true
+       }
+    },
     {  "id": "issue11931",
        "file": "pdfs/issue11931.pdf",
        "md5": "9ea233037992e1f10280420a49e72845",

test/unit/annotation_spec.js

@@ -1882,11 +1882,11 @@ describe("annotation", function () {
       buttonWidgetDict.set("V", Name.get("1"));
 
       const appearanceStatesDict = new Dict();
-      const exportValueOptionsDict = new Dict();
+      const normalAppearanceDict = new Dict();
 
-      exportValueOptionsDict.set("Off", 0);
+      normalAppearanceDict.set("Off", 0);
-      exportValueOptionsDict.set("Checked", 1);
+      normalAppearanceDict.set("Checked", 1);
-      appearanceStatesDict.set("D", exportValueOptionsDict);
+      appearanceStatesDict.set("N", normalAppearanceDict);
       buttonWidgetDict.set("AP", appearanceStatesDict);
 
       const buttonWidgetRef = Ref.get(124, 0);
@@ -1931,9 +1931,38 @@ describe("annotation", function () {
       }, done.fail);
     });
 
+    it("should handle checkboxes without /Off appearance", function (done) {
+      buttonWidgetDict.set("V", Name.get("1"));
+
+      const appearanceStatesDict = new Dict();
+      const normalAppearanceDict = new Dict();
+
+      normalAppearanceDict.set("Checked", 1);
+      appearanceStatesDict.set("N", normalAppearanceDict);
+      buttonWidgetDict.set("AP", appearanceStatesDict);
+
+      const buttonWidgetRef = Ref.get(124, 0);
+      const xref = new XRefMock([
+        { ref: buttonWidgetRef, data: buttonWidgetDict },
+      ]);
+
+      AnnotationFactory.create(
+        xref,
+        buttonWidgetRef,
+        pdfManagerMock,
+        idFactoryMock
+      ).then(({ data }) => {
+        expect(data.annotationType).toEqual(AnnotationType.WIDGET);
+        expect(data.checkBox).toEqual(true);
+        expect(data.fieldValue).toEqual("1");
+        expect(data.radioButton).toEqual(false);
+        expect(data.exportValue).toEqual("Checked");
+        done();
+      }, done.fail);
+    });
+
     it("should render checkboxes for printing", function (done) {
       const appearanceStatesDict = new Dict();
-      const exportValueOptionsDict = new Dict();
       const normalAppearanceDict = new Dict();
       const checkedAppearanceDict = new Dict();
       const uncheckedAppearanceDict = new Dict();
@@ -1949,9 +1978,6 @@ describe("annotation", function () {
       checkedAppearanceDict.set("Matrix", [1, 0, 0, 1, 0, 0]);
       normalAppearanceDict.set("Checked", checkedStream);
       normalAppearanceDict.set("Off", uncheckedStream);
-      exportValueOptionsDict.set("Off", 0);
-      exportValueOptionsDict.set("Checked", 1);
-      appearanceStatesDict.set("D", exportValueOptionsDict);
       appearanceStatesDict.set("N", normalAppearanceDict);
 
       buttonWidgetDict.set("AP", appearanceStatesDict);
@@ -2019,14 +2045,10 @@ describe("annotation", function () {
 
     it("should save checkboxes", function (done) {
       const appearanceStatesDict = new Dict();
-      const exportValueOptionsDict = new Dict();
       const normalAppearanceDict = new Dict();
 
       normalAppearanceDict.set("Checked", Ref.get(314, 0));
       normalAppearanceDict.set("Off", Ref.get(271, 0));
-      exportValueOptionsDict.set("Off", 0);
-      exportValueOptionsDict.set("Checked", 1);
-      appearanceStatesDict.set("D", exportValueOptionsDict);
       appearanceStatesDict.set("N", normalAppearanceDict);
 
       buttonWidgetDict.set("AP", appearanceStatesDict);
@@ -2059,8 +2081,7 @@ describe("annotation", function () {
           expect(oldData.data).toEqual(
             "123 0 obj\n" +
               "<< /Type /Annot /Subtype /Widget /FT /Btn " +
-              "/AP << /D << /Off 0 /Checked 1>> " +
+              "/AP << /N << /Checked 314 0 R /Off 271 0 R>>>> " +
-              "/N << /Checked 314 0 R /Off 271 0 R>>>> " +
               "/V /Checked /AS /Checked /M (date)>>\nendobj\n"
           );
           return annotation;
@@ -2142,7 +2163,6 @@ describe("annotation", function () {
 
     it("should render radio buttons for printing", function (done) {
       const appearanceStatesDict = new Dict();
-      const exportValueOptionsDict = new Dict();
       const normalAppearanceDict = new Dict();
       const checkedAppearanceDict = new Dict();
       const uncheckedAppearanceDict = new Dict();
@@ -2158,9 +2178,6 @@ describe("annotation", function () {
       checkedAppearanceDict.set("Matrix", [1, 0, 0, 1, 0, 0]);
       normalAppearanceDict.set("Checked", checkedStream);
       normalAppearanceDict.set("Off", uncheckedStream);
-      exportValueOptionsDict.set("Off", 0);
-      exportValueOptionsDict.set("Checked", 1);
-      appearanceStatesDict.set("D", exportValueOptionsDict);
       appearanceStatesDict.set("N", normalAppearanceDict);
 
       buttonWidgetDict.set("Ff", AnnotationFieldFlag.RADIO);
@@ -2229,14 +2246,10 @@ describe("annotation", function () {
 
     it("should save radio buttons", function (done) {
       const appearanceStatesDict = new Dict();
-      const exportValueOptionsDict = new Dict();
       const normalAppearanceDict = new Dict();
 
       normalAppearanceDict.set("Checked", Ref.get(314, 0));
       normalAppearanceDict.set("Off", Ref.get(271, 0));
-      exportValueOptionsDict.set("Off", 0);
-      exportValueOptionsDict.set("Checked", 1);
-      appearanceStatesDict.set("D", exportValueOptionsDict);
       appearanceStatesDict.set("N", normalAppearanceDict);
 
       buttonWidgetDict.set("Ff", AnnotationFieldFlag.RADIO);
@@ -2282,8 +2295,7 @@ describe("annotation", function () {
           expect(radioData.data).toEqual(
             "123 0 obj\n" +
               "<< /Type /Annot /Subtype /Widget /FT /Btn /Ff 32768 " +
-              "/AP << /D << /Off 0 /Checked 1>> " +
+              "/AP << /N << /Checked 314 0 R /Off 271 0 R>>>> " +
-              "/N << /Checked 314 0 R /Off 271 0 R>>>> " +
               "/Parent 456 0 R /AS /Checked /M (date)>>\nendobj\n"
           );
           expect(parentData.ref).toEqual(Ref.get(456, 0));
@@ -2450,16 +2462,12 @@ describe("annotation", function () {
       }, done.fail);
     });
 
-    it("should sanitize display values in option arrays (issue 8947)", function (done) {
+    it("should decode form values", function (done) {
-      // The option value is a UTF-16BE string. The display value should be
+      const encodedString = "\xFE\xFF\x00F\x00o\x00o";
-      // sanitized, but the export value should remain the same since that
+      const decodedString = "Foo";
-      // may be used as a unique identifier when exporting form values.
-      const options = ["\xFE\xFF\x00F\x00o\x00o"];
-      const expected = [
-        { exportValue: "\xFE\xFF\x00F\x00o\x00o", displayValue: "Foo" },
-      ];
 
-      choiceWidgetDict.set("Opt", options);
+      choiceWidgetDict.set("Opt", [encodedString]);
+      choiceWidgetDict.set("V", encodedString);
 
       const choiceWidgetRef = Ref.get(984, 0);
       const xref = new XRefMock([
@@ -2473,7 +2481,10 @@ describe("annotation", function () {
         idFactoryMock
       ).then(({ data }) => {
         expect(data.annotationType).toEqual(AnnotationType.WIDGET);
-        expect(data.options).toEqual(expected);
+        expect(data.fieldValue).toEqual([decodedString]);
+        expect(data.options).toEqual([
+          { exportValue: decodedString, displayValue: decodedString },
+        ]);
         done();
       }, done.fail);
     });