From b6e60d033a204686c78f695c2324523598fa92b5 Mon Sep 17 00:00:00 2001 From: Calixte Denizet Date: Fri, 4 Oct 2024 17:07:25 +0200 Subject: [PATCH] [Editor] Avoid an exception when undoing the deletion of a pre-existing annotation It fixes #18849. When such an annotation is deleted, we make sure that there are some data to restore. The version of this patch was making undoing a svg deletion buggy, so it's fixed now and an integration test has been added for this case. --- src/display/editor/stamp.js | 3 + src/display/editor/tools.js | 25 ++++++- test/integration/stamp_editor_spec.mjs | 97 ++++++++++++++++++++++++-- 3 files changed, 117 insertions(+), 8 deletions(-) diff --git a/src/display/editor/stamp.js b/src/display/editor/stamp.js index ae631ef55a84f..a2d0ff3dc6f71 100644 --- 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 index db755c08bd5e7..9d4fd87476720 100644 --- 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 index 2f3b683c4b710..b7603a0f99db9 100644 --- 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,8 +145,6 @@ describe("Stamp Editor", () => { const [bitmap] = await serializeBitmapDimensions(page); expect(bitmap.width).toEqual(512); expect(bitmap.height).toEqual(543); - - await clearAll(page); }) ); }); @@ -138,14 +152,15 @@ describe("Stamp Editor", () => { 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); + }) + ); + }); + }); });