Merge pull request #21674 from calixteman/touch/fix-swallowed-pointerup

Let pointerup/cancel propagate in TouchManager

src/display/touch_manager.js

@@ -15,6 +15,10 @@
 
 import { OutputScale, stopEvent } from "./display_utils.js";
 
+function preventDefault(evt) {
+  evt.preventDefault();
+}
+
 class TouchManager {
   #container;
 
@@ -135,8 +139,13 @@ class TouchManager {
       opt.capture = true;
       container.addEventListener("pointerdown", stopEvent, opt);
       container.addEventListener("pointermove", stopEvent, opt);
-      container.addEventListener("pointercancel", stopEvent, opt);
-      container.addEventListener("pointerup", stopEvent, opt);
+      // `pointerup` and `pointercancel` are only default-prevented: a
+      // `stopPropagation` in the capture phase also skips the bubble-phase
+      // listeners of the very node it's called on, hence swallowing them here
+      // would prevent any session in flight, e.g. an editor being resized, from
+      // ever being ended.
+      container.addEventListener("pointercancel", preventDefault, opt);
+      container.addEventListener("pointerup", preventDefault, opt);
       this.#onPinchStart?.();
     }
 

test/integration/ink_editor_spec.mjs

@@ -36,6 +36,7 @@ import {
   selectEditor,
   selectEditors,
   switchToEditor,
+  unselectEditor,
   waitForAnnotationModeChanged,
   waitForNoElement,
   waitForPointerUp,
@@ -66,6 +67,26 @@ const drawLine = async (page, x0, y0, x1, y1) => {
   await awaitPromise(clickHandle);
 };
 
+// Draw an editor large enough to have room for two fingers on it, and leave it
+// selected since that's what makes it resizable with a touchscreen.
+const drawAndSelectEditor = async page => {
+  await switchToInk(page);
+  const { x, y, width, height } = await getRect(page, ".annotationEditorLayer");
+  await drawLine(
+    page,
+    x + 0.15 * width,
+    y + 0.1 * height,
+    x + 0.75 * width,
+    y + 0.35 * height
+  );
+  await commit(page);
+
+  return {
+    layer: { x, y, width, height },
+    editor: await getRect(page, getEditorSelector(0)),
+  };
+};
+
 describe("Ink Editor", () => {
   describe("Basic operations", () => {
     let pages;
@@ -1445,13 +1466,12 @@ describe("Pinch to resize a drawing", () => {
         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,
+        await pinch(page, {
+          centerX: before.x + before.width / 2,
+          centerY: before.y + before.height / 2,
           startGap,
-          endGap
-        );
+          endGap,
+        });
         const { width: after } = await getRect(page, editorSelector);
 
         expect(after)
@@ -1461,3 +1481,96 @@ describe("Pinch to resize a drawing", () => {
     );
   });
 });
+
+describe("Resize with a touchscreen", () => {
+  let pages;
+
+  beforeEach(async () => {
+    pages = await loadAndWait("empty.pdf", ".annotationEditorLayer");
+  });
+
+  afterEach(async () => {
+    await closePages(pages);
+  });
+
+  it("must check that the resize session is ended when the finger is lifted while another one is down", async () => {
+    await Promise.all(
+      pages.map(async ([browserName, page]) => {
+        const { layer } = await drawAndSelectEditor(page);
+
+        // Grabbing a resizer starts a resize session, which disables the
+        // pointer events of the editor layer until the finger is lifted.
+        const resizer = await getRect(
+          page,
+          `${getEditorSelector(0)} .resizer.bottomRight`
+        );
+        await pinch(page, {
+          steps: 0,
+          startPoints: [
+            {
+              x: resizer.x + resizer.width / 2,
+              y: resizer.y + resizer.height / 2,
+            },
+            {
+              x: layer.x + 0.3 * layer.width,
+              y: layer.y + 0.9 * layer.height,
+            },
+          ],
+          afterFirstStart: () =>
+            page.waitForSelector(".annotationEditorLayer.disabled"),
+          afterFirstEnd: () =>
+            // The `pointerup` of the resizing finger must still be dispatched,
+            // else the resize session would never end and the editor would keep
+            // being resized by the plain mouse moves coming afterwards.
+            page.waitForSelector(".annotationEditorLayer:not(.disabled)"),
+        });
+      })
+    );
+  });
+});
+
+describe("Tap after a two-finger gesture", () => {
+  let pages;
+
+  beforeEach(async () => {
+    pages = await loadAndWait("empty.pdf", ".annotationEditorLayer");
+  });
+
+  afterEach(async () => {
+    await closePages(pages);
+  });
+
+  it("must check that the tap following a two-finger gesture selects the editor", async () => {
+    await Promise.all(
+      pages.map(async ([browserName, page]) => {
+        const { layer, editor } = await drawAndSelectEditor(page);
+        const editorSelector = getEditorSelector(0);
+
+        // One finger on the editor, which starts a drag session, and a second
+        // one elsewhere on the page, which turns it into a two-finger gesture.
+        await pinch(page, {
+          steps: 0,
+          startPoints: [
+            {
+              x: editor.x + 0.5 * editor.width,
+              y: editor.y + 0.8 * editor.height,
+            },
+            {
+              x: layer.x + 0.3 * layer.width,
+              y: layer.y + 0.9 * layer.height,
+            },
+          ],
+        });
+
+        // Once every finger is up, no listener may be left behind to swallow
+        // the `pointerdown` of the next tap on the editor.
+        await unselectEditor(page, editorSelector);
+        await page.touchscreen.tap(
+          editor.x + 0.5 * editor.width,
+          editor.y + 0.8 * editor.height
+        );
+        await waitForSelectedEditor(page, editorSelector);
+      })
+    );
+  });
+});

test/integration/test_utils.mjs

@@ -606,23 +606,69 @@ async function dragAndDrop(page, selector, translations, steps = 1) {
 // 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);
+// Explicit start/end points can be used for tests which need an asymmetric
+// gesture, or hooks around each touch lifetime.
+async function pinch(
+  page,
+  {
+    afterEnd = null,
+    afterFirstEnd = null,
+    afterFirstStart = null,
+    afterStart = null,
+    beforeEnd = null,
+    centerX = 0,
+    centerY = 0,
+    centerDeltaX = 0,
+    centerDeltaY = 0,
+    startGap = 0,
+    endGap = startGap,
+    endPoints = null,
+    startPoints = null,
+    steps = 12,
   }
+) {
+  const normalizePoint = point =>
+    Array.isArray(point) ? { x: point[0], y: point[1] } : point;
+  const start = (
+    startPoints || [
+      { x: centerX - startGap, y: centerY },
+      { x: centerX + startGap, y: centerY },
+    ]
+  ).map(normalizePoint);
+  let end;
+  if (endPoints) {
+    end = endPoints.map(normalizePoint);
+  } else if (startPoints) {
+    end = start;
+  } else {
+    end = [
+      { x: centerX + centerDeltaX - endGap, y: centerY + centerDeltaY },
+      { x: centerX + centerDeltaX + endGap, y: centerY + centerDeltaY },
+    ];
+  }
+
+  const finger0 = await page.touchscreen.touchStart(start[0].x, start[0].y);
+  await afterFirstStart?.(finger0);
+  const finger1 = await page.touchscreen.touchStart(start[1].x, start[1].y);
+  await afterStart?.([finger0, finger1]);
+
+  for (let i = 1; i <= steps; i++) {
+    const t = i / steps;
+    await finger0.move(
+      start[0].x + (end[0].x - start[0].x) * t,
+      start[0].y + (end[0].y - start[0].y) * t
+    );
+    await finger1.move(
+      start[1].x + (end[1].x - start[1].x) * t,
+      start[1].y + (end[1].y - start[1].y) * t
+    );
+  }
+
+  await beforeEnd?.([finger0, finger1]);
   await finger0.end();
+  await afterFirstEnd?.([finger0, finger1]);
   await finger1.end();
+  await afterEnd?.([finger0, finger1]);
 }
 
 function waitForPageChanging(page) {

test/integration/viewer_spec.mjs

@@ -1561,7 +1561,12 @@ describe("PDF viewer", () => {
           // 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 pinch(page, {
+            centerX: originX,
+            centerY: originY,
+            startGap: 25,
+            endGap: 100,
+          });
           await awaitPromise(rendered);
 
           const spanHandle = await page.evaluateHandle(() =>