From 8960eddfc165fa29506e074f5b73e7ef306504d6 Mon Sep 17 00:00:00 2001 From: Calixte Denizet Date: Tue, 28 Jul 2026 22:38:05 +0200 Subject: [PATCH] Use the client coordinates for the pinch-zoom origin `TouchManager` computes the pinch distances with the screen coordinates, but it was passing the pinch center in that space too, while the viewer expects a client coordinate, like the origin coming from a wheel event. Since a two-finger gesture can be synthesized with WebDriver BiDi, the pinch-zoom test doesn't have to rely on CDP anymore and can run in Chrome, where it fails without the above fix. --- src/display/touch_manager.js | 9 ++++-- test/integration/ink_editor_spec.mjs | 38 ++++++++-------------- test/integration/test_utils.mjs | 26 +++++++++++++++ test/integration/viewer_spec.mjs | 48 ++++++++++++++++------------ web/pdf_viewer.js | 11 ++++--- 5 files changed, 80 insertions(+), 52 deletions(-) diff --git a/src/display/touch_manager.js b/src/display/touch_manager.js index 08de46001..290f2da93 100644 --- a/src/display/touch_manager.js +++ b/src/display/touch_manager.js @@ -189,7 +189,7 @@ class TouchManager { const pDistance = Math.hypot(prevGapX, prevGapY) || 1; if ( !this.#isPinching && - Math.abs(pDistance - distance) <= TouchManager.MIN_TOUCH_DISTANCE_TO_PINCH + Math.abs(pDistance - distance) <= this.MIN_TOUCH_DISTANCE_TO_PINCH ) { return; } @@ -207,7 +207,12 @@ class TouchManager { return; } - const origin = [(screen0X + screen1X) / 2, (screen0Y + screen1Y) / 2]; + // The distances are in screen CSS pixels, but the origin must be in client + // coordinates, like the one coming from a wheel event. + const origin = [ + (touch0.clientX + touch1.clientX) / 2, + (touch0.clientY + touch1.clientY) / 2, + ]; this.#onPinching?.(origin, pDistance, distance); } diff --git a/test/integration/ink_editor_spec.mjs b/test/integration/ink_editor_spec.mjs index b9d2eb9e8..7c968e809 100644 --- a/test/integration/ink_editor_spec.mjs +++ b/test/integration/ink_editor_spec.mjs @@ -31,6 +31,7 @@ import { kbUndo, loadAndWait, moveEditor, + pinch, scrollIntoView, selectEditor, selectEditors, @@ -1419,30 +1420,9 @@ describe("Pinch to resize a drawing", () => { await closePages(pages); }); - // Spread two fingers apart, centered on the editor. - async function pinchOut(page, selector) { - const { x, y, width, height } = await getRect(page, selector); - const centerX = x + width / 2; - const centerY = y + height / 2; - const finger0 = await page.touchscreen.touchStart(centerX - 25, centerY); - const finger1 = await page.touchscreen.touchStart(centerX + 25, centerY); - for (let i = 1; i <= 12; i++) { - const gap = 25 + i * 12; - await finger0.move(centerX - gap, centerY); - await finger1.move(centerX + gap, centerY); - } - await finger0.end(); - await finger1.end(); - } - it("must keep resizing a drawing which came back with an undo", async () => { await Promise.all( pages.map(async ([browserName, page]) => { - if (browserName === "firefox") { - pending( - "Touch events are not supported on devices without touch screen in Firefox." - ); - } await switchToInk(page); const rect = await getRect(page, ".annotationEditorLayer"); @@ -1462,11 +1442,21 @@ describe("Pinch to resize a drawing", () => { await page.waitForSelector(editorSelector); await selectEditor(page, editorSelector); - const { width: before } = await getRect(page, editorSelector); - await pinchOut(page, editorSelector); + const before = await getRect(page, editorSelector); + const startGap = Math.min(before.width, before.height) * 0.2; + const endGap = Math.max(before.width, before.height) * 1.8; + await pinch( + page, + before.x + before.width / 2, + before.y + before.height / 2, + startGap, + endGap + ); const { width: after } = await getRect(page, editorSelector); - expect(after).withContext(`In ${browserName}`).toBeGreaterThan(before); + expect(after) + .withContext(`In ${browserName}`) + .toBeGreaterThan(before.width); }) ); }); diff --git a/test/integration/test_utils.mjs b/test/integration/test_utils.mjs index 8aab655d6..d31f479ca 100644 --- a/test/integration/test_utils.mjs +++ b/test/integration/test_utils.mjs @@ -600,6 +600,31 @@ async function dragAndDrop(page, selector, translations, steps = 1) { await page.waitForSelector("#viewer:not(.noUserSelect)"); } +// Move two fingers, horizontally centered on (centerX, centerY), from startGap +// to endGap: it's a pinch out when endGap is larger than startGap. +// Keep in mind that `TouchManager` starts to pinch only once the distance +// between the two fingers changed by more than `MIN_TOUCH_DISTANCE_TO_PINCH` +// (35 CSS pixels), hence the first moves are swallowed and the resulting zoom +// factor is smaller than endGap / startGap. +async function pinch(page, centerX, centerY, startGap, endGap, steps = 12) { + const finger0 = await page.touchscreen.touchStart( + centerX - startGap, + centerY + ); + const finger1 = await page.touchscreen.touchStart( + centerX + startGap, + centerY + ); + const delta = (endGap - startGap) / steps; + for (let i = 1; i <= steps; i++) { + const gap = startGap + i * delta; + await finger0.move(centerX - gap, centerY); + await finger1.move(centerX + gap, centerY); + } + await finger0.end(); + await finger1.end(); +} + function waitForPageChanging(page) { return createPromise(page, resolve => { window.PDFViewerApplication.eventBus.on("pagechanging", resolve, { @@ -1163,6 +1188,7 @@ export { paste, pasteFromClipboard, PDI, + pinch, scrollIntoView, selectEditor, selectEditors, diff --git a/test/integration/viewer_spec.mjs b/test/integration/viewer_spec.mjs index 4ef44035a..8dac75505 100644 --- a/test/integration/viewer_spec.mjs +++ b/test/integration/viewer_spec.mjs @@ -20,6 +20,7 @@ import { getRect, getSpanRectFromText, loadAndWait, + pinch, scrollIntoView, showViewsManager, waitAndClick, @@ -1527,7 +1528,11 @@ describe("PDF viewer", () => { beforeEach(async () => { pages = await loadAndWait( "tracemonkey.pdf", - `.page[data-page-number = "1"] .endOfContent` + `.page[data-page-number = "1"] .endOfContent`, + // Pin the zoom: the drift checked below is proportional to the zoom + // level reached at the end of the pinch, and the default `page-fit` + // depends on the size of the window. + 50 ); }); @@ -1535,20 +1540,9 @@ describe("PDF viewer", () => { await closePages(pages); }); - it("keeps the content under the pinch centre fixed on the screen", async () => { + it("keeps the content under the pinch center fixed on the screen", async () => { await Promise.all( pages.map(async ([browserName, page]) => { - if (browserName === "firefox") { - pending( - "Touch events are not supported on devices without touch screen in Firefox." - ); - } - if (browserName === "chrome") { - pending( - "Pinch zoom emulation is not supported for WebDriver BiDi in Chrome." - ); - } - const rect = await getSpanRectFromText(page, 1, "type-stable"); const originX = rect.x + rect.width / 2; const originY = rect.y + rect.height / 2; @@ -1564,14 +1558,12 @@ describe("PDF viewer", () => { }; window.PDFViewerApplication.eventBus.on("textlayerrendered", cb); }); - const client = await page.target().createCDPSession(); - await client.send("Input.synthesizePinchGesture", { - x: originX, - y: originY, - scaleFactor: 3, - gestureSourceType: "touch", - }); + // Spread the two fingers from 50 to 200 pixels apart: the first + // moves are swallowed until the distance between them changed by + // more than 35 pixels, hence a zoom factor of about 200/85 = 2.4. + await pinch(page, originX, originY, 25, 100); await awaitPromise(rendered); + const spanHandle = await page.evaluateHandle(() => Array.from( document.querySelectorAll( @@ -1579,7 +1571,21 @@ describe("PDF viewer", () => { ) ).find(span => span.textContent.includes("type-stable")) ); - expect(await spanHandle.isIntersectingViewport()).toBeTrue(); + expect(await spanHandle.isIntersectingViewport()) + .withContext(`In ${browserName}`) + .toBeTrue(); + + // The text which was under the fingers must still be at the same + // height: only vertically because a page which is larger than its + // container isn't centered in it anymore. + // A few pixels are tolerated because the origin is preserved by + // scrolling: Chrome snaps the scroll offsets to the device pixels and + // the discarded fractions show up as a small drift. It's exact in + // Firefox, which keeps them. + const newRect = await getSpanRectFromText(page, 1, "type-stable"); + expect(Math.abs(newRect.y + newRect.height / 2 - originY)) + .withContext(`In ${browserName}`) + .toBeLessThan(5); }) ); }); diff --git a/web/pdf_viewer.js b/web/pdf_viewer.js index 268888c61..2d261618c 100644 --- a/web/pdf_viewer.js +++ b/web/pdf_viewer.js @@ -1890,19 +1890,20 @@ class PDFViewer { container.scrollLeft - firstPage.x, container.scrollTop - firstPage.y ); - const intLeft = Math.round(topLeft[0]); - const intTop = Math.round(topLeft[1]); + const [left, top] = topLeft; let pdfOpenParams = `#page=${pageNumber}`; if (!this.isInPresentationMode) { - pdfOpenParams += `&zoom=${normalizedScaleValue},${intLeft},${intTop}`; + pdfOpenParams += + `&zoom=${normalizedScaleValue},` + + `${Math.round(left)},${Math.round(top)}`; } this._location = { pageNumber, scale: normalizedScaleValue, - top: intTop, - left: intLeft, + top, + left, rotation: this._pagesRotation, pdfOpenParams, };