ref #431 Merge breaks form field rendering/formatting in Mac Preview

Only removes key/value pairs from the widget dict when the acroFormPolicy is MERGE; no longer removes for MERGE_RENAMING and FLATTEN.

sejda-sambox/src/main/java/org/sejda/impl/sambox/component/AcroFormsMerger.java

@@ -249,14 +249,14 @@ public void mergeForm(PDAcroForm originalForm, LookupTable<PDAnnotation> annotat
             switch (policy) {
             case MERGE_RENAMING_EXISTING_FIELDS:
                 updateForm(originalForm, annotationsLookup, createRenamingTerminalField,
-                        createRenamingNonTerminalField);
+                        createRenamingNonTerminalField, false);
                 break;
             case MERGE:
-                updateForm(originalForm, annotationsLookup, createOrReuseTerminalField, createOrReuseNonTerminalField);
+                updateForm(originalForm, annotationsLookup, createOrReuseTerminalField, createOrReuseNonTerminalField, true);
                 break;
             case FLATTEN:
                 updateForm(originalForm, annotationsLookup, createRenamingTerminalField,
-                        createRenamingNonTerminalField);
+                        createRenamingNonTerminalField, false);
                 flatten();
                 break;
             default:
@@ -286,7 +286,7 @@ private void removeFieldKeysFromWidgets(Collection<PDAnnotationWidget> annotatio
 
     private void updateForm(PDAcroForm originalForm, LookupTable<PDAnnotation> annotationsLookup,
             BiFunction<PDTerminalField, LookupTable<PDField>, PDTerminalField> getTerminalField,
-            BiConsumer<PDField, LookupTable<PDField>> createNonTerminalField) {
+            BiConsumer<PDField, LookupTable<PDField>> createNonTerminalField, boolean shouldRemoveFieldKeysFromWidgets) {
         AcroFormUtils.mergeDefaults(originalForm, form);
         LookupTable<PDField> fieldsLookup = new LookupTable<>();
         Set<PDAnnotationWidget> allRelevantWidgets = annotationsLookup.keys().stream()
@@ -297,7 +297,7 @@ private void updateForm(PDAcroForm originalForm, LookupTable<PDAnnotation> annot
         // every widget we meet is removed from the allRelevantWidgets so we can identify widgets not referenced by the originalForm
         originalForm.getFieldTree().stream().forEach(
                 f -> mergeField(f, annotationsLookup, getTerminalField, createNonTerminalField, fieldsLookup,
-                        of(allRelevantWidgets::remove)));
+                        of(allRelevantWidgets::remove), shouldRemoveFieldKeysFromWidgets));
         // keep track of the root fields
         originalForm.getFields().stream().map(fieldsLookup::lookup).filter(Objects::nonNull).forEach(rootFields::add);
 
@@ -316,7 +316,7 @@ private void updateForm(PDAcroForm originalForm, LookupTable<PDAnnotation> annot
             });
 
             dummy.getFieldTree().stream().forEach(f -> mergeField(f, annotationsLookup, getTerminalField,
-                    createNonTerminalField, fieldsLookup, empty()));
+                    createNonTerminalField, fieldsLookup, empty(), shouldRemoveFieldKeysFromWidgets));
             // keep track of the root fields
             dummy.getFields().stream().map(fieldsLookup::lookup).filter(Objects::nonNull).forEach(rootFields::add);
         }
@@ -329,15 +329,21 @@ private void updateForm(PDAcroForm originalForm, LookupTable<PDAnnotation> annot
     private void mergeField(PDField field, LookupTable<PDAnnotation> annotationsLookup,
             BiFunction<PDTerminalField, LookupTable<PDField>, PDTerminalField> getTerminalField,
             BiConsumer<PDField, LookupTable<PDField>> createNonTerminalField, LookupTable<PDField> fieldsLookup,
-            Optional<Consumer<PDAnnotationWidget>> onProcessedWidget) {
+            Optional<Consumer<PDAnnotationWidget>> onProcessedWidget, boolean shouldRemoveFieldKeysFromWidgets) {
         if (!field.isTerminal()) {
             createNonTerminalField.accept(field, fieldsLookup);
         } else {
             List<PDAnnotationWidget> relevantWidgets = findMappedWidgetsFor((PDTerminalField) field, annotationsLookup);
             if (!relevantWidgets.isEmpty()) {
                 PDTerminalField terminalField = getTerminalField.apply((PDTerminalField) field, fieldsLookup);
                 if (nonNull(terminalField)) {
-                    removeFieldKeysFromWidgets(relevantWidgets);
+                    // removal is no longer performed for MERGE_RENAMING or FLATTEN policies
+                    // we currently only perform this removal when policy is MERGE (backwards compatibility)
+                    // TODO: review if this removal should be peformed for the MERGE policy
+                    if(shouldRemoveFieldKeysFromWidgets) {
+                        removeFieldKeysFromWidgets(relevantWidgets);
+                    }
+
                     for (PDAnnotationWidget widget : relevantWidgets) {
                         terminalField.addWidgetIfMissing(widget);
                         onProcessedWidget.ifPresent(c -> field.getWidgets().forEach(c));

sejda-sambox/src/test/java/org/sejda/impl/sambox/MergeSamboxTaskTest.java

@@ -40,12 +40,15 @@
 import org.sejda.model.scale.PageNormalizationPolicy;
 import org.sejda.model.task.Task;
 import org.sejda.model.toc.ToCPolicy;
+import org.sejda.sambox.cos.COSName;
 import org.sejda.sambox.pdmodel.PDDocument;
 import org.sejda.sambox.pdmodel.PDPage;
 import org.sejda.sambox.pdmodel.PageNotFoundException;
 import org.sejda.sambox.pdmodel.common.PDPageLabelRange;
 import org.sejda.sambox.pdmodel.common.PDPageLabels;
 import org.sejda.sambox.pdmodel.common.PDRectangle;
+import org.sejda.sambox.pdmodel.interactive.annotation.PDAnnotation;
+import org.sejda.sambox.pdmodel.interactive.annotation.PDAnnotationWidget;
 import org.sejda.sambox.pdmodel.interactive.form.PDField;
 import org.sejda.sambox.text.PDFTextStripperByArea;
 import org.sejda.tests.DocBuilder;
@@ -1088,8 +1091,28 @@ public void withLargeTocItemsThatWrapAndGenerateMultipleTocPages() throws IOExce
             assertFooterHasText(d.getPage(27), "Attachment 26 - Sample file name - shorter 28");
         });
     }
+
+    @Test
+    public void form_field_loses_formatting_in_Mac_Preview() throws IOException {
+        MergeParameters parameters = new MergeParameters();
+        parameters.setExistingOutputPolicy(ExistingOutputPolicy.OVERWRITE);
+        parameters.addInput(new PdfMergeInput(customInput("pdf/forms/form_field_centered_text.pdf")));
+        parameters.setAcroFormPolicy(AcroFormPolicy.MERGE_RENAMING_EXISTING_FIELDS);
+
+        testContext.pdfOutputTo(parameters);
+        execute(parameters);
+
+        testContext.assertTaskCompleted();
+        testContext.assertPages(1).forEachPdfOutput(d -> {
+            PDPage page = d.getPage(0);
+            PDAnnotation annotation = page.getAnnotations().get(0);
+            PDAnnotationWidget widget = (PDAnnotationWidget) annotation;
+            assertEquals(1, widget.getCOSObject().getInt(COSName.Q));
+            assertEquals("/Helv 8 Tf 0.718 0.11 0.11 rg", widget.getCOSObject().getString(COSName.DA));
+        });
+    }
 
-    private float widthOfCropBox(PDPage page) {
+            private float widthOfCropBox(PDPage page) {
         return page.getCropBox().rotate(page.getRotation()).getWidth();
     }