Merge pull request #18851 from calixteman/issue18849

[Editor] Avoid an exception when undoing the deletion of a pre-existing annotation
mozilla · Oct 4, 2024 · 7c1883a · 7c1883a
2 parents e5b6144 + b6e60d0
commit 7c1883a
Show file tree

Hide file tree

Showing 3 changed files with 117 additions and 8 deletions.
diff --git a/src/display/editor/stamp.js b/src/display/editor/stamp.js
@@ -837,6 +837,9 @@ class StampEditor extends AnnotationEditor {
       editor.altTextData = accessibilityData;
     }
     editor._initialData = initialData;
+    // No need to be add in the undo stack if the editor is created from an
+    // existing one.
+    editor.#hasBeenAddedInUndoStack = !!initialData;
 
     return editor;
   }

diff --git a/src/display/editor/tools.js b/src/display/editor/tools.js
@@ -131,8 +131,10 @@ class ImageManager {
       if (typeof rawData === "string") {
         data.url = rawData;
         image = await fetchData(rawData, "blob");
-      } else {
+      } else if (rawData instanceof File) {
         image = data.file = rawData;
+      } else if (rawData instanceof Blob) {
+        image = rawData;
       }
 
       if (image.type === "image/svg+xml") {
@@ -183,6 +185,11 @@ class ImageManager {
     return this.#get(url, url);
   }
 
+  async getFromBlob(id, blobPromise) {
+    const blob = await blobPromise;
+    return this.#get(id, blob);
+  }
+
   async getFromId(id) {
     this.#cache ||= new Map();
     const data = this.#cache.get(id);
@@ -197,6 +204,11 @@ class ImageManager {
     if (data.file) {
       return this.getFromFile(data.file);
     }
+    if (data.blobPromise) {
+      const { blobPromise } = data;
+      delete data.blobPromise;
+      return this.getFromBlob(data.id, blobPromise);
+    }
     return this.getFromUrl(data.url);
   }
 
@@ -239,7 +251,16 @@ class ImageManager {
     if (data.refCounter !== 0) {
       return;
     }
-    data.bitmap.close?.();
+    const { bitmap } = data;
+    if (!data.url && !data.file) {
+      // The image has no way to be restored (ctrl+z) so we must fix that.
+      const canvas = new OffscreenCanvas(bitmap.width, bitmap.height);
+      const ctx = canvas.getContext("bitmaprenderer");
+      ctx.transferFromImageBitmap(bitmap);
+      data.blobPromise = canvas.convertToBlob();
+    }
+
+    bitmap.close?.();
     data.bitmap = null;
   }
 

diff --git a/test/integration/stamp_editor_spec.mjs b/test/integration/stamp_editor_spec.mjs
@@ -101,7 +101,23 @@ describe("Stamp Editor", () => {
     let pages;
 
     beforeAll(async () => {
-      pages = await loadAndWait("empty.pdf", ".annotationEditorLayer");
+      pages = await loadAndWait("empty.pdf", ".annotationEditorLayer", null, {
+        eventBusSetup: eventBus => {
+          eventBus.on("annotationeditoruimanager", ({ uiManager }) => {
+            window.uiManager = uiManager;
+          });
+        },
+      });
+    });
+
+    afterEach(async () => {
+      for (const [, page] of pages) {
+        await page.evaluate(() => {
+          window.uiManager.reset();
+        });
+        // Disable editing mode.
+        await switchToStamp(page, /* disable */ true);
+      }
     });
 
     afterAll(async () => {
@@ -129,23 +145,22 @@ describe("Stamp Editor", () => {
           const [bitmap] = await serializeBitmapDimensions(page);
           expect(bitmap.width).toEqual(512);
           expect(bitmap.height).toEqual(543);
-
-          await clearAll(page);
         })
       );
     });
 
     it("must load a SVG", async () => {
       await Promise.all(
         pages.map(async ([browserName, page]) => {
+          await switchToStamp(page);
           await page.click("#editorStampAddImage");
           const input = await page.$("#stampEditorFileInput");
           await input.uploadFile(
             `${path.join(__dirname, "../images/firefox_logo.svg")}`
           );
-          await waitForImage(page, getEditorSelector(1));
+          await waitForImage(page, getEditorSelector(0));
 
-          const { width } = await getEditorDimensions(page, 1);
+          const { width } = await getEditorDimensions(page, 0);
 
           expect(Math.round(parseFloat(width))).toEqual(40);
 
@@ -157,8 +172,32 @@ describe("Stamp Editor", () => {
           );
           expect(Math.abs(bitmap.width - 242 * ratio) < 1).toBeTrue();
           expect(Math.abs(bitmap.height - 80 * ratio) < 1).toBeTrue();
+        })
+      );
+    });
+
+    it("must load a SVG, delete it and undo", async () => {
+      await Promise.all(
+        pages.map(async ([browserName, page]) => {
+          await switchToStamp(page);
+          await page.click("#editorStampAddImage");
+          const input = await page.$("#stampEditorFileInput");
+          await input.uploadFile(
+            `${path.join(__dirname, "../images/firefox_logo.svg")}`
+          );
+          const editorSelector = getEditorSelector(0);
+          await waitForImage(page, editorSelector);
+
+          await waitForSerialized(page, 1);
+          await page.waitForSelector(`${editorSelector} button.delete`);
+          await page.click(`${editorSelector} button.delete`);
+
+          await waitForSerialized(page, 0);
 
-          await clearAll(page);
+          await kbUndo(page);
+          await waitForSerialized(page, 1);
+
+          await waitForSelectedEditor(page, editorSelector);
         })
       );
     });
@@ -1388,4 +1427,50 @@ describe("Stamp Editor", () => {
       );
     });
   });
+
+  describe("Stamp (delete existing and undo)", () => {
+    let pages;
+
+    beforeAll(async () => {
+      pages = await loadAndWait("stamps.pdf", getAnnotationSelector("37R"));
+    });
+
+    afterAll(async () => {
+      await closePages(pages);
+    });
+
+    it("must check that the annotation is correctly restored", async () => {
+      await Promise.all(
+        pages.map(async ([browserName, page]) => {
+          await page.click(getAnnotationSelector("37R"), { count: 2 });
+          const editorSelector = getEditorSelector(2);
+          await waitForSelectedEditor(page, editorSelector);
+
+          const editorIds = await getEditors(page, "stamp");
+          expect(editorIds.length).withContext(`In ${browserName}`).toEqual(5);
+
+          // All the current annotations should be serialized as null objects
+          // because they haven't been edited yet.
+          let serialized = await getSerialized(page);
+          expect(serialized).withContext(`In ${browserName}`).toEqual([]);
+
+          await page.waitForSelector(`${editorSelector} button.delete`);
+          await page.click(`${editorSelector} button.delete`);
+
+          await waitForSerialized(page, 1);
+          serialized = await getSerialized(page);
+          expect(serialized)
+            .withContext(`In ${browserName}`)
+            .toEqual([
+              { id: "37R", deleted: true, pageIndex: 0, popupRef: "44R" },
+            ]);
+
+          await kbUndo(page);
+          await waitForSerialized(page, 0);
+
+          await waitForSelectedEditor(page, editorSelector);
+        })
+      );
+    });
+  });
 });