[Editor] When deleting an annotation with popup, then delete the popu…

…p too
mozilla · Sep 26, 2024 · 0382dd0 · 0382dd0
1 parent 3902a14
commit 0382dd0
Show file tree

Hide file tree

Showing 9 changed files with 104 additions and 25 deletions.
diff --git a/src/core/document.js b/src/core/document.js
@@ -284,6 +284,12 @@ class Page {
         }
         if (annotation.deleted) {
           deletedAnnotations.put(ref, ref);
+          if (annotation.popupRef) {
+            const popupRef = Ref.fromString(annotation.popupRef);
+            if (popupRef) {
+              deletedAnnotations.put(popupRef, popupRef);
+            }
+          }
           continue;
         }
         existingAnnotations?.put(ref);

diff --git a/src/display/editor/editor.js b/src/display/editor/editor.js
@@ -80,6 +80,8 @@ class AnnotationEditor {
 
   _initialOptions = Object.create(null);
 
+  _initialData = null;
+
   _isVisible = true;
 
   _uiManager = null;
@@ -1335,6 +1337,19 @@ class AnnotationEditor {
    */
   rotate(_angle) {}
 
+  /**
+   * Serialize the editor when it has been deleted.
+   * @returns {Object}
+   */
+  serializeDeleted() {
+    return {
+      id: this.annotationElementId,
+      deleted: true,
+      pageIndex: this.pageIndex,
+      popupRef: this._initialData?.popupRef || "",
+    };
+  }
+
   /**
    * Serialize the editor.
    * The result of the serialization will be used to construct a
@@ -1809,11 +1824,7 @@ class FakeEditor extends AnnotationEditor {
   }
 
   serialize() {
-    return {
-      id: this.annotationElementId,
-      deleted: true,
-      pageIndex: this.pageIndex,
-    };
+    return this.serializeDeleted();
   }
 }
 

diff --git a/src/display/editor/freetext.js b/src/display/editor/freetext.js
@@ -48,8 +48,6 @@ class FreeTextEditor extends AnnotationEditor {
 
   #fontSize;
 
-  #initialData = null;
-
   static _freeTextDefaultContent = "";
 
   static _internalPadding = 0;
@@ -598,7 +596,7 @@ class FreeTextEditor extends AnnotationEditor {
 
         // position is the position of the first glyph in the annotation
         // and it's relative to its container.
-        const { position } = this.#initialData;
+        const { position } = this._initialData;
         let [tx, ty] = this.getInitialTranslation();
         [tx, ty] = this.pageTranslationToScreen(tx, ty);
         const [pageWidth, pageHeight] = this.pageDimensions;
@@ -781,6 +779,7 @@ class FreeTextEditor extends AnnotationEditor {
           rect,
           rotation,
           id,
+          popupRef,
         },
         textContent,
         textPosition,
@@ -805,14 +804,15 @@ class FreeTextEditor extends AnnotationEditor {
         rotation,
         id,
         deleted: false,
+        popupRef,
       };
     }
     const editor = super.deserialize(data, parent, uiManager);
     editor.#fontSize = data.fontSize;
     editor.#color = Util.makeHexColor(...data.color);
     editor.#content = FreeTextEditor.#deserializeContent(data.value);
     editor.annotationElementId = data.id || null;
-    editor.#initialData = initialData;
+    editor._initialData = initialData;
 
     return editor;
   }
@@ -824,11 +824,7 @@ class FreeTextEditor extends AnnotationEditor {
     }
 
     if (this.deleted) {
-      return {
-        pageIndex: this.pageIndex,
-        id: this.annotationElementId,
-        deleted: true,
-      };
+      return this.serializeDeleted();
     }
 
     const padding = FreeTextEditor._internalPadding * this.parentScale;
@@ -866,7 +862,7 @@ class FreeTextEditor extends AnnotationEditor {
   }
 
   #hasElementChanged(serialized) {
-    const { value, fontSize, color, pageIndex } = this.#initialData;
+    const { value, fontSize, color, pageIndex } = this._initialData;
 
     return (
       this._hasBeenMoved ||

diff --git a/src/display/editor/highlight.js b/src/display/editor/highlight.js
@@ -55,8 +55,6 @@ class HighlightEditor extends AnnotationEditor {
 
   #id = null;
 
-  #initialData = null;
-
   #isFreeHighlight = false;
 
   #lastPoint = null;
@@ -785,7 +783,7 @@ class HighlightEditor extends AnnotationEditor {
     let initialData = null;
     if (data instanceof HighlightAnnotationElement) {
       const {
-        data: { quadPoints, rect, rotation, id, color, opacity },
+        data: { quadPoints, rect, rotation, id, color, opacity, popupRef },
         parent: {
           page: { pageNumber },
         },
@@ -801,6 +799,7 @@ class HighlightEditor extends AnnotationEditor {
         rotation,
         id,
         deleted: false,
+        popupRef,
       };
     } else if (data instanceof InkAnnotationElement) {
       const {
@@ -811,6 +810,7 @@ class HighlightEditor extends AnnotationEditor {
           id,
           color,
           borderStyle: { rawWidth: thickness },
+          popupRef,
         },
         parent: {
           page: { pageNumber },
@@ -827,6 +827,7 @@ class HighlightEditor extends AnnotationEditor {
         rotation,
         id,
         deleted: false,
+        popupRef,
       };
     }
 
@@ -839,7 +840,7 @@ class HighlightEditor extends AnnotationEditor {
       editor.#thickness = data.thickness;
     }
     editor.annotationElementId = data.id || null;
-    editor.#initialData = initialData;
+    editor._initialData = initialData;
 
     const [pageWidth, pageHeight] = editor.pageDimensions;
     const [pageX, pageY] = editor.pageTranslation;
@@ -902,11 +903,7 @@ class HighlightEditor extends AnnotationEditor {
     }
 
     if (this.deleted) {
-      return {
-        pageIndex: this.pageIndex,
-        id: this.annotationElementId,
-        deleted: true,
-      };
+      return this.serializeDeleted();
     }
 
     const rect = this.getRect(0, 0);
@@ -934,7 +931,7 @@ class HighlightEditor extends AnnotationEditor {
   }
 
   #hasElementChanged(serialized) {
-    const { color } = this.#initialData;
+    const { color } = this._initialData;
     return serialized.color.some((c, i) => c !== color[i]);
   }
 

diff --git a/test/integration/freetext_editor_spec.mjs b/test/integration/freetext_editor_spec.mjs
@@ -1237,6 +1237,7 @@ describe("FreeText Editor", () => {
               pageIndex: 0,
               id: "51R",
               deleted: true,
+              popupRef: "",
             },
           ]);
 

diff --git a/test/integration/highlight_editor_spec.mjs b/test/integration/highlight_editor_spec.mjs
@@ -1972,6 +1972,50 @@ describe("Highlight Editor", () => {
     });
   });
 
+  describe("Highlight (delete an existing annotation)", () => {
+    let pages;
+
+    beforeAll(async () => {
+      pages = await loadAndWait(
+        "highlight_popup.pdf",
+        ".annotationEditorLayer"
+      );
+    });
+
+    afterAll(async () => {
+      await closePages(pages);
+    });
+
+    it("must delete an existing annotation and its popup", async () => {
+      await Promise.all(
+        pages.map(async ([browserName, page]) => {
+          const modeChangedHandle = await waitForAnnotationModeChanged(page);
+          await waitAndClick(page, "[data-annotation-id='24R']", { count: 2 });
+          await awaitPromise(modeChangedHandle);
+          await page.waitForSelector("#highlightParamsToolbarContainer");
+
+          const editorSelector = getEditorSelector(0);
+          await page.waitForSelector(editorSelector);
+          await page.waitForSelector(`${editorSelector} button.delete`);
+          await page.click(`${editorSelector} button.delete`);
+          await waitForSerialized(page, 1);
+
+          const serialized = await getSerialized(page);
+          expect(serialized)
+            .withContext(`In ${browserName}`)
+            .toEqual([
+              {
+                pageIndex: 0,
+                id: "24R",
+                deleted: true,
+                popupRef: "25R",
+              },
+            ]);
+        })
+      );
+    });
+  });
+
   describe("Free Highlight (edit existing in double clicking on it)", () => {
     let pages;
 

diff --git a/test/pdfs/.gitignore b/test/pdfs/.gitignore
@@ -670,3 +670,4 @@
 !bug1918115.pdf
 !bug1919513.pdf
 !issue16038.pdf
+!highlight_popup.pdf
diff --git a/test/pdfs/highlight_popup.pdf b/test/pdfs/highlight_popup.pdf
diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js
@@ -2926,6 +2926,29 @@ describe("api", function () {
       await loadingTask.destroy();
     });
 
+    it("write an highlight annotation and delete its popup", async function () {
+      let loadingTask = getDocument(
+        buildGetDocumentParams("highlight_popup.pdf")
+      );
+      let pdfDoc = await loadingTask.promise;
+      pdfDoc.annotationStorage.setValue("pdfjs_internal_editor_0", {
+        deleted: true,
+        id: "24R",
+        pageIndex: 0,
+        popupRef: "25R",
+      });
+      const data = await pdfDoc.saveDocument();
+      await loadingTask.destroy();
+
+      loadingTask = getDocument(data);
+      pdfDoc = await loadingTask.promise;
+      const page = await pdfDoc.getPage(1);
+      const annotations = await page.getAnnotations();
+
+      expect(annotations).toEqual([]);
+      await loadingTask.destroy();
+    });
+
     it("read content from multiline textfield containing an empty line", async function () {
       const loadingTask = getDocument(buildGetDocumentParams("issue17492.pdf"));
       const pdfDoc = await loadingTask.promise;