Merge pull request #16556 from calixteman/no_dup_when_saving

[Editor] Avoid to have duplicated entries in the Annot array when saving an existing and modified annotation
mozilla · Jun 15, 2023 · 8937cac · 8937cac
2 parents 5f1fba5 + 71479fd
commit 8937cac
Show file tree

Hide file tree

Showing 4 changed files with 93 additions and 4 deletions.
diff --git a/src/core/document.js b/src/core/document.js
@@ -258,7 +258,7 @@ class Page {
     );
   }
 
-  #replaceIdByRef(annotations, deletedAnnotations) {
+  #replaceIdByRef(annotations, deletedAnnotations, existingAnnotations) {
     for (const annotation of annotations) {
       if (annotation.id) {
         const ref = Ref.fromString(annotation.id);
@@ -270,6 +270,7 @@ class Page {
           deletedAnnotations.put(ref);
           continue;
         }
+        existingAnnotations?.put(ref);
         annotation.ref = ref;
         delete annotation.id;
       }
@@ -295,7 +296,8 @@ class Page {
     });
 
     const deletedAnnotations = new RefSet();
-    this.#replaceIdByRef(annotations, deletedAnnotations);
+    const existingAnnotations = new RefSet();
+    this.#replaceIdByRef(annotations, deletedAnnotations, existingAnnotations);
 
     const pageDict = this.pageDict;
     const annotationsArray = this.annotations.filter(
@@ -308,7 +310,10 @@ class Page {
     );
 
     for (const { ref } of newData.annotations) {
-      annotationsArray.push(ref);
+      // Don't add an existing annotation ref to the annotations array.
+      if (ref instanceof Ref && !existingAnnotations.has(ref)) {
+        annotationsArray.push(ref);
+      }
     }
 
     const savedDict = pageDict.get("Annots");
@@ -431,7 +436,7 @@ class Page {
       const newAnnotations = newAnnotationsByPage.get(this.pageIndex);
       if (newAnnotations) {
         deletedAnnotations = new RefSet();
-        this.#replaceIdByRef(newAnnotations, deletedAnnotations);
+        this.#replaceIdByRef(newAnnotations, deletedAnnotations, null);
         newAnnotationsPromise = AnnotationFactory.printNewAnnotations(
           partialEvaluator,
           task,

diff --git a/src/core/worker.js b/src/core/worker.js
@@ -823,6 +823,11 @@ class WorkerMessageHandler {
           .ensureXRef("trailer")
           .then(trailer => trailer.get("Prev"));
       });
+      handler.on("GetAnnotArray", function (data) {
+        return pdfManager.getPage(data.pageIndex).then(function (page) {
+          return page.annotations.map(a => a.toString());
+        });
+      });
     }
 
     return workerHandlerName;

diff --git a/src/display/api.js b/src/display/api.js
@@ -780,6 +780,11 @@ class PDFDocumentProxy {
           return this._transport.getXRefPrevValue();
         },
       });
+      Object.defineProperty(this, "getAnnotArray", {
+        value: pageIndex => {
+          return this._transport.getAnnotArray(pageIndex);
+        },
+      });
     }
   }
 
@@ -2405,6 +2410,13 @@ class WorkerTransport {
           return this.messageHandler.sendWithPromise("GetXRefPrevValue", null);
         },
       });
+      Object.defineProperty(this, "getAnnotArray", {
+        value: pageIndex => {
+          return this.messageHandler.sendWithPromise("GetAnnotArray", {
+            pageIndex,
+          });
+        },
+      });
     }
   }
 

diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js
@@ -2008,6 +2008,73 @@ describe("api", function () {
       await loadingTask.destroy();
     });
 
+    it("edit and write an existing annotation, save the pdf and check that the Annot array doesn't contain dup entries", async function () {
+      let loadingTask = getDocument(buildGetDocumentParams("issue14438.pdf"));
+      let pdfDoc = await loadingTask.promise;
+      pdfDoc.annotationStorage.setValue("pdfjs_internal_editor_0", {
+        annotationType: AnnotationEditorType.FREETEXT,
+        rect: [12, 34, 56, 78],
+        rotation: 0,
+        fontSize: 10,
+        color: [0, 0, 0],
+        value: "Hello PDF.js World!",
+        pageIndex: 0,
+        id: "10R",
+      });
+      pdfDoc.annotationStorage.setValue("pdfjs_internal_editor_1", {
+        annotationType: AnnotationEditorType.FREETEXT,
+        rect: [12, 34, 56, 78],
+        rotation: 0,
+        fontSize: 10,
+        color: [0, 0, 0],
+        value: "Hello PDF.js World!",
+        pageIndex: 0,
+      });
+
+      const data = await pdfDoc.saveDocument();
+      await loadingTask.destroy();
+
+      loadingTask = getDocument(data);
+      pdfDoc = await loadingTask.promise;
+      const annotations = await pdfDoc.getAnnotArray(0);
+
+      expect(annotations).toEqual([
+        "4R",
+        "10R",
+        "17R",
+        "20R",
+        "21R",
+        "22R",
+        "25R",
+        "28R",
+        "29R",
+        "30R",
+        "33R",
+        "36R",
+        "37R",
+        "42R",
+        "43R",
+        "44R",
+        "47R",
+        "50R",
+        "51R",
+        "54R",
+        "55R",
+        "58R",
+        "59R",
+        "62R",
+        "63R",
+        "66R",
+        "69R",
+        "72R",
+        "75R",
+        "78R",
+        "140R",
+      ]);
+
+      await loadingTask.destroy();
+    });
+
     describe("Cross-origin", function () {
       let loadingTask;
       function _checkCanLoad(expectSuccess, filename, options) {