Skip to content

Commit

Permalink
ref #429: handle inconsistent /P on annotations following spec, which…
Browse files Browse the repository at this point in the history 
… says /P on annotations is optional (set to null), except for 'screen' subtypes (set to rendered page);
  • Loading branch information
@ediweissmann
ediweissmann committed Dec 27, 2023
1 parent 3de0cac commit 4a56540
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 21 deletions.
32 changes: 16 additions & 16 deletions sejda-sambox/src/main/java/org/sejda/impl/sambox/component/AnnotationsDistiller.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,19 +78,21 @@ public LookupTable<PDAnnotation> retainRelevantAnnotations(LookupTable<PDPage> r
keptAnnotations.add(mapped);
} else {
// handle the scenario where the annotation page != actual render page
// TODO: for safety, applying this only to form widgets for now
if(annotation instanceof PDAnnotationWidget) {
PDPage annotationPage = annotation.getPage();
if (annotationPage != null && !annotationPage.equals(page)) {
// inconsistent annotation, this creates problems
PDPage annotationPage = annotation.getPage();
if (annotationPage != null && !annotationPage.equals(page)) {
// inconsistent annotation, this creates problems; let's fix it
if(COSName.SCREEN.equals(annotation.getSubtype())) {
annotation.setPage(page);
} else {
annotation.setPage(null);
}
}


if (annotation instanceof PDAnnotationLink) {
processLinkAnnotation(relevantPages, keptAnnotations, (PDAnnotationLink) annotation);
processLinkAnnotation(relevantPages, keptAnnotations, (PDAnnotationLink) annotation, page);
} else {
processNonLinkAnnotation(relevantPages, keptAnnotations, annotation);
processNonLinkAnnotation(relevantPages, keptAnnotations, annotation, page);
}
}
}
Expand All @@ -103,13 +105,13 @@ public LookupTable<PDAnnotation> retainRelevantAnnotations(LookupTable<PDPage> r
}

private void processLinkAnnotation(LookupTable<PDPage> relevantPages, Set<PDAnnotation> keptAnnotations,
PDAnnotationLink annotation) throws IOException {
PDAnnotationLink annotation, PDPage p) throws IOException {
PDPageDestination destination = getDestinationFrom(annotation);
if (nonNull(destination)) {
PDPage destPage = relevantPages.lookup(destination.getPage());
if (nonNull(destPage)) {
// relevant page dest
PDAnnotationLink duplicate = (PDAnnotationLink) duplicate(annotation, relevantPages);
PDAnnotationLink duplicate = (PDAnnotationLink) duplicate(annotation, p, relevantPages);
duplicate.getCOSObject().removeItem(COSName.A);
PDPageDestination newDestination = (PDPageDestination) PDDestination
.create(destination.getCOSObject().duplicate());
Expand All @@ -121,23 +123,22 @@ private void processLinkAnnotation(LookupTable<PDPage> relevantPages, Set<PDAnno
}
} else {
// not a page dest
keptAnnotations.add(duplicate(annotation, relevantPages));
keptAnnotations.add(duplicate(annotation, p, relevantPages));
}
}

private void processNonLinkAnnotation(LookupTable<PDPage> relevantPages, Set<PDAnnotation> keptAnnotations,
PDAnnotation annotation) {
PDPage p = annotation.getPage();
PDAnnotation annotation, PDPage p) {
if (isNull(p) || relevantPages.hasLookupFor(p)) {
PDAnnotation duplicate = duplicate(annotation, relevantPages);
PDAnnotation duplicate = duplicate(annotation, p, relevantPages);
if (duplicate instanceof PDAnnotationMarkup) {
PDAnnotationPopup popup = ((PDAnnotationMarkup) duplicate).getPopup();
if (nonNull(popup)) {
COSName subtype = popup.getCOSObject().getCOSName(COSName.SUBTYPE);
if (COSName.POPUP.equals(subtype)) {
PDAnnotationPopup popupDuplicate = ofNullable(
(PDAnnotationPopup) annotationsLookup.lookup(popup))
.orElseGet(() -> (PDAnnotationPopup) duplicate(popup, relevantPages));
.orElseGet(() -> (PDAnnotationPopup) duplicate(popup, p, relevantPages));
((PDAnnotationMarkup) duplicate).setPopup(popupDuplicate);
if (nonNull(popupDuplicate.getParent())) {
popupDuplicate.setParent((PDAnnotationMarkup) duplicate);
Expand All @@ -154,8 +155,7 @@ private void processNonLinkAnnotation(LookupTable<PDPage> relevantPages, Set<PDA
}
}

private PDAnnotation duplicate(PDAnnotation annotation, LookupTable<PDPage> relevantPages) {
PDPage p = annotation.getPage();
private PDAnnotation duplicate(PDAnnotation annotation, PDPage p, LookupTable<PDPage> relevantPages) {
PDAnnotation duplicate = PDAnnotation.createAnnotation(annotation.getCOSObject().duplicate());
if (nonNull(p)) {
duplicate.setPage(relevantPages.lookup(p));
Expand Down
9 changes: 4 additions & 5 deletions sejda-sambox/src/test/java/org/sejda/impl/sambox/component/AnnotationsDistillerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,8 @@ public void noLinks_PageNotRelevant() {
doc.addPage(oldPage);
LookupTable<PDAnnotation> annotationsLookup = new AnnotationsDistiller(doc).retainRelevantAnnotations(lookup);

// TODO: review this test; the annotation should be present in the merged result, even if /P is inconsistent
assertEquals(0, newPage.getAnnotations().size());
assertTrue(annotationsLookup.isEmpty());
assertEquals(1, newPage.getAnnotations().size());
assertFalse(annotationsLookup.isEmpty());
}

@Test
Expand All @@ -154,8 +153,8 @@ public void noLinks_OnePageNotRelevantOneRelevant() {
doc.addPage(oldPage);
LookupTable<PDAnnotation> annotationsLookup = new AnnotationsDistiller(doc).retainRelevantAnnotations(lookup);

// TODO: review this test; the fist annotation should be present in the merged result, even if /P is inconsistent
assertEquals(annotationsLookup.lookup(annotation2), newPage.getAnnotations().get(0));
assertEquals(annotationsLookup.lookup(annotation), newPage.getAnnotations().get(0));
assertEquals(annotationsLookup.lookup(annotation2), newPage.getAnnotations().get(1));
}

@Test
Expand Down

0 comments on commit 4a56540

Please sign in to comment.