From 99f3e6b4ec3003a80971135869a36f787abc65f3 Mon Sep 17 00:00:00 2001 From: Calixte Denizet Date: Mon, 9 Dec 2024 16:07:39 +0100 Subject: [PATCH] [Editor] When resizing a stamp annotation, the opposite corner must stay fixed It was due the resize observer which is removed thanks to this patch. In order to reuse the dragAndDrop function in test, this patch slighty refactors it in order to make it easier to use. --- src/display/editor/stamp.js | 109 +++++++++------------- test/integration/freetext_editor_spec.mjs | 31 +++--- test/integration/ink_editor_spec.mjs | 10 +- test/integration/stamp_editor_spec.mjs | 92 ++++++++++++------ test/integration/test_utils.mjs | 22 ++++- 5 files changed, 139 insertions(+), 125 deletions(-) diff --git a/src/display/editor/stamp.js b/src/display/editor/stamp.js index 972e009ca..b727a0baf 100644 --- a/src/display/editor/stamp.js +++ b/src/display/editor/stamp.js @@ -40,8 +40,6 @@ class StampEditor extends AnnotationEditor { #canvas = null; - #observer = null; - #resizeTimeoutId = null; #isSvg = false; @@ -305,8 +303,6 @@ class StampEditor extends AnnotationEditor { this._uiManager.imageManager.deleteId(this.#bitmapId); this.#canvas?.remove(); this.#canvas = null; - this.#observer?.disconnect(); - this.#observer = null; if (this.#resizeTimeoutId) { clearTimeout(this.#resizeTimeoutId); this.#resizeTimeoutId = null; @@ -398,9 +394,34 @@ class StampEditor extends AnnotationEditor { ); } + this._uiManager.addShouldRescale(this); + return this.div; } + /** @inheritdoc */ + _onResized() { + // We used a CSS-zoom during the resizing, but now it's resized we can + // rescale correctly the bitmap to fit the new dimensions. + this.onScaleChanging(); + } + + onScaleChanging() { + if (!this.parent) { + return; + } + if (this.#resizeTimeoutId !== null) { + clearTimeout(this.#resizeTimeoutId); + } + // The user's zooming the page, there is no need to redraw the bitmap at + // each step, hence we wait a bit before redrawing it. + const TIME_TO_WAIT = 200; + this.#resizeTimeoutId = setTimeout(() => { + this.#resizeTimeoutId = null; + this.#drawBitmap(); + }, TIME_TO_WAIT); + } + #createCanvas() { const { div } = this; let { width, height } = this.#bitmap; @@ -433,6 +454,15 @@ class StampEditor extends AnnotationEditor { canvas.setAttribute("role", "img"); this.addContainer(canvas); + this.width = width / pageWidth; + this.height = height / pageHeight; + if (this._initialOptions?.isCentered) { + this.center(); + } else { + this.fixAndSetPosition(); + } + this._initialOptions = null; + if ( !this._uiManager.useNewAltTextWhenAddingImage || !this._uiManager.useNewAltTextFlow || @@ -440,8 +470,7 @@ class StampEditor extends AnnotationEditor { ) { div.hidden = false; } - this.#drawBitmap(width, height); - this.#createObserver(); + this.#drawBitmap(); if (!this.#hasBeenAddedInUndoStack) { this.parent.addUndoableEditor(this); this.#hasBeenAddedInUndoStack = true; @@ -584,37 +613,6 @@ class StampEditor extends AnnotationEditor { return { canvas, width, height, imageData }; } - /** - * When the dimensions of the div change the inner canvas must - * renew its dimensions, hence it must redraw its own contents. - * @param {number} width - the new width of the div - * @param {number} height - the new height of the div - * @returns - */ - #setDimensions(width, height) { - const [parentWidth, parentHeight] = this.parentDimensions; - this.width = width / parentWidth; - this.height = height / parentHeight; - if (this._initialOptions?.isCentered) { - this.center(); - } else { - this.fixAndSetPosition(); - } - this._initialOptions = null; - if (this.#resizeTimeoutId !== null) { - clearTimeout(this.#resizeTimeoutId); - } - // When the user is resizing the editor we just use CSS to scale the image - // to avoid redrawing it too often. - // And once the user stops resizing the editor we redraw the image in - // rescaling it correctly (see this.#scaleBitmap). - const TIME_TO_WAIT = 200; - this.#resizeTimeoutId = setTimeout(() => { - this.#resizeTimeoutId = null; - this.#drawBitmap(width, height); - }, TIME_TO_WAIT); - } - #scaleBitmap(width, height) { const { width: bitmapWidth, height: bitmapHeight } = this.#bitmap; @@ -660,18 +658,21 @@ class StampEditor extends AnnotationEditor { return bitmap; } - #drawBitmap(width, height) { + #drawBitmap() { + const [parentWidth, parentHeight] = this.parentDimensions; + const { width, height } = this; const outputScale = new OutputScale(); - const scaledWidth = Math.ceil(width * outputScale.sx); - const scaledHeight = Math.ceil(height * outputScale.sy); - + const scaledWidth = Math.ceil(width * parentWidth * outputScale.sx); + const scaledHeight = Math.ceil(height * parentHeight * outputScale.sy); const canvas = this.#canvas; + if ( !canvas || (canvas.width === scaledWidth && canvas.height === scaledHeight) ) { return; } + canvas.width = scaledWidth; canvas.height = scaledHeight; @@ -746,32 +747,6 @@ class StampEditor extends AnnotationEditor { return structuredClone(this.#bitmap); } - /** - * Create the resize observer. - */ - #createObserver() { - if (!this._uiManager._signal) { - // This method is called after the canvas has been created but the canvas - // creation is async, so it's possible that the viewer has been closed. - return; - } - this.#observer = new ResizeObserver(entries => { - const rect = entries[0].contentRect; - if (rect.width && rect.height) { - this.#setDimensions(rect.width, rect.height); - } - }); - this.#observer.observe(this.div); - this._uiManager._signal.addEventListener( - "abort", - () => { - this.#observer?.disconnect(); - this.#observer = null; - }, - { once: true } - ); - } - /** @inheritdoc */ static async deserialize(data, parent, uiManager) { let initialData = null; diff --git a/test/integration/freetext_editor_spec.mjs b/test/integration/freetext_editor_spec.mjs index 085c75e6a..ca47dea9f 100644 --- a/test/integration/freetext_editor_spec.mjs +++ b/test/integration/freetext_editor_spec.mjs @@ -19,7 +19,7 @@ import { copy, copyToClipboard, createPromise, - dragAndDropAnnotation, + dragAndDrop, firstPageOnTop, getEditors, getEditorSelector, @@ -954,19 +954,14 @@ describe("FreeText Editor", () => { const serialized = await getSerialized(page); expect(serialized).withContext(`In ${browserName}`).toEqual([]); - const editorRect = await getRect(page, getEditorSelector(0)); + const editorSelector = getEditorSelector(0); + const editorRect = await getRect(page, editorSelector); // Select the annotation we want to move. await page.mouse.click(editorRect.x + 2, editorRect.y + 2); - await waitForSelectedEditor(page, getEditorSelector(0)); + await waitForSelectedEditor(page, editorSelector); - await dragAndDropAnnotation( - page, - editorRect.x + editorRect.width / 2, - editorRect.y + editorRect.height / 2, - 100, - 100 - ); + await dragAndDrop(page, editorSelector, [[100, 100]]); await waitForSerialized(page, 1); }) ); @@ -2335,29 +2330,29 @@ describe("FreeText Editor", () => { const allPositions = []; for (let i = 0; i < 10; i++) { + const editorSelector = getEditorSelector(i); await page.mouse.click(rect.x + 10 + 30 * i, rect.y + 100 + 5 * i); - await page.waitForSelector(getEditorSelector(i), { + await page.waitForSelector(editorSelector, { visible: true, }); await page.type( - `${getEditorSelector(i)} .internal`, + `${editorSelector} .internal`, String.fromCharCode(65 + i) ); // Commit. await page.keyboard.press("Escape"); - await page.waitForSelector( - `${getEditorSelector(i)} .overlay.enabled` - ); + await page.waitForSelector(`${editorSelector} .overlay.enabled`); - allPositions.push(await getRect(page, getEditorSelector(i))); + allPositions.push(await getRect(page, editorSelector)); } await selectAll(page); - await dragAndDropAnnotation(page, rect.x + 161, rect.y + 126, 39, 74); + await dragAndDrop(page, getEditorSelector(4), [[39, 74]]); for (let i = 0; i < 10; i++) { - const pos = await getRect(page, getEditorSelector(i)); + const editorSelector = getEditorSelector(i); + const pos = await getRect(page, editorSelector); const oldPos = allPositions[i]; expect(Math.abs(Math.round(pos.x - oldPos.x) - 39)) .withContext(`In ${browserName}`) diff --git a/test/integration/ink_editor_spec.mjs b/test/integration/ink_editor_spec.mjs index 93f7de906..9ba625416 100644 --- a/test/integration/ink_editor_spec.mjs +++ b/test/integration/ink_editor_spec.mjs @@ -17,7 +17,7 @@ import { awaitPromise, closePages, createPromise, - dragAndDropAnnotation, + dragAndDrop, getAnnotationSelector, getEditors, getEditorSelector, @@ -822,13 +822,7 @@ describe("Ink Editor", () => { await page.mouse.click(editorRect.x + 2, editorRect.y + 2); await waitForSelectedEditor(page, edgeB); - await dragAndDropAnnotation( - page, - editorRect.x + editorRect.width / 2, - editorRect.y + editorRect.height / 2, - 100, - 100 - ); + await dragAndDrop(page, edgeB, [[100, 100]]); await waitForSerialized(page, 1); }) ); diff --git a/test/integration/stamp_editor_spec.mjs b/test/integration/stamp_editor_spec.mjs index 6e76a45b9..acbc13a62 100644 --- a/test/integration/stamp_editor_spec.mjs +++ b/test/integration/stamp_editor_spec.mjs @@ -16,11 +16,12 @@ import { applyFunctionToEditor, awaitPromise, + cleanupEditing, clearInput, closePages, copy, copyToClipboard, - dragAndDropAnnotation, + dragAndDrop, getAnnotationSelector, getEditorDimensions, getEditors, @@ -120,13 +121,7 @@ describe("Stamp Editor", () => { }); afterEach(async () => { - for (const [, page] of pages) { - await page.evaluate(() => { - window.uiManager.reset(); - }); - // Disable editing mode. - await switchToStamp(page, /* disable */ true); - } + await cleanupEditing(pages, switchToStamp); }); afterAll(async () => { @@ -216,13 +211,23 @@ describe("Stamp Editor", () => { let pages; beforeAll(async () => { - pages = await loadAndWait("empty.pdf", ".annotationEditorLayer", 50); + pages = await loadAndWait("empty.pdf", ".annotationEditorLayer", 50, { + eventBusSetup: eventBus => { + eventBus.on("annotationeditoruimanager", ({ uiManager }) => { + window.uiManager = uiManager; + }); + }, + }); }); afterAll(async () => { await closePages(pages); }); + afterEach(async () => { + await cleanupEditing(pages, switchToStamp); + }); + it("must check that an added image stay within the page", async () => { await Promise.all( pages.map(async ([browserName, page]) => { @@ -239,14 +244,13 @@ describe("Stamp Editor", () => { await input.uploadFile( `${path.join(__dirname, "../images/firefox_logo.png")}` ); - await waitForImage(page, getEditorSelector(i)); - await page.waitForSelector(`${getEditorSelector(i)} .altText`); + const editorSelector = getEditorSelector(i); + await waitForImage(page, editorSelector); + await page.waitForSelector(`${editorSelector} .altText`); for (let j = 0; j < 4; j++) { await page.keyboard.press("Escape"); - await page.waitForSelector( - `${getEditorSelector(i)} .resizers.hidden` - ); + await page.waitForSelector(`${editorSelector} .resizers.hidden`); const handle = await waitForAnnotationEditorLayer(page); await page.evaluate(() => { @@ -255,10 +259,10 @@ describe("Stamp Editor", () => { await awaitPromise(handle); await page.focus(".stampEditor"); - await waitForSelectedEditor(page, getEditorSelector(i)); + await waitForSelectedEditor(page, editorSelector); await page.waitForSelector( - `${getEditorSelector(i)} .resizers:not(.hidden)` + `${editorSelector} .resizers:not(.hidden)` ); const stampRect = await getRect(page, ".stampEditor"); @@ -285,6 +289,44 @@ describe("Stamp Editor", () => { }) ); }); + + it("must check that the opposite corner doesn't move", 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.png")}` + ); + const editorSelector = getEditorSelector(0); + await waitForImage(page, editorSelector); + await page.waitForSelector(`${editorSelector} .resizer.topLeft`); + const baseRect = await getRect(page, editorSelector); + const bRX = baseRect.x + baseRect.width; + const bRY = baseRect.y + baseRect.height; + + await dragAndDrop(page, `${editorSelector} .resizer.topLeft`, [ + [-10, -10], + [20, 20], + [-10, -10], + [20, 20], + ]); + + const newRect = await getRect(page, editorSelector); + const newBRX = newRect.x + newRect.width; + const newBRY = newRect.y + newRect.height; + + expect(Math.abs(bRX - newBRX) <= 1) + .withContext(`In ${browserName}`) + .toBeTrue(); + expect(Math.abs(bRY - newBRY) <= 1) + .withContext(`In ${browserName}`) + .toBeTrue(); + }) + ); + }); }); describe("Alt text dialog", () => { @@ -1441,7 +1483,8 @@ describe("Stamp Editor", () => { const modeChangedHandle = await waitForAnnotationModeChanged(page); await page.click(getAnnotationSelector("25R"), { count: 2 }); await awaitPromise(modeChangedHandle); - await waitForSelectedEditor(page, getEditorSelector(0)); + const editorSelector = getEditorSelector(0); + await waitForSelectedEditor(page, editorSelector); const editorIds = await getEditors(page, "stamp"); expect(editorIds.length).withContext(`In ${browserName}`).toEqual(5); @@ -1451,22 +1494,13 @@ describe("Stamp Editor", () => { const serialized = await getSerialized(page); expect(serialized).withContext(`In ${browserName}`).toEqual([]); - const editorRect = await page.$eval(getEditorSelector(0), el => { - const { x, y, width, height } = el.getBoundingClientRect(); - return { x, y, width, height }; - }); + const editorRect = await getRect(page, editorSelector); // Select the annotation we want to move. await page.mouse.click(editorRect.x + 2, editorRect.y + 2); - await waitForSelectedEditor(page, getEditorSelector(0)); + await waitForSelectedEditor(page, editorSelector); - await dragAndDropAnnotation( - page, - editorRect.x + editorRect.width / 2, - editorRect.y + editorRect.height / 2, - 100, - 100 - ); + await dragAndDrop(page, editorSelector, [[100, 100]]); await waitForSerialized(page, 1); }) ); diff --git a/test/integration/test_utils.mjs b/test/integration/test_utils.mjs index c52f5257b..f30d81d57 100644 --- a/test/integration/test_utils.mjs +++ b/test/integration/test_utils.mjs @@ -522,10 +522,15 @@ async function serializeBitmapDimensions(page) { }); } -async function dragAndDropAnnotation(page, startX, startY, tX, tY) { +async function dragAndDrop(page, selector, translations) { + const rect = await getRect(page, selector); + const startX = rect.x + rect.width / 2; + const startY = rect.y + rect.height / 2; await page.mouse.move(startX, startY); await page.mouse.down(); - await page.mouse.move(startX + tX, startY + tY); + for (const [tX, tY] of translations) { + await page.mouse.move(startX + tX, startY + tY); + } await page.mouse.up(); await page.waitForSelector("#viewer:not(.noUserSelect)"); } @@ -809,16 +814,27 @@ function isCanvasWhite(page, pageNumber, rectangle) { ); } +async function cleanupEditing(pages, switcher) { + for (const [, page] of pages) { + await page.evaluate(() => { + window.uiManager.reset(); + }); + // Disable editing mode. + await switcher(page, /* disable */ true); + } +} + export { applyFunctionToEditor, awaitPromise, + cleanupEditing, clearInput, closePages, closeSinglePage, copy, copyToClipboard, createPromise, - dragAndDropAnnotation, + dragAndDrop, firstPageOnTop, getAnnotationSelector, getAnnotationStorage,