From a1cd62fbc92115132537fb89244d5b6ecbcac7a4 Mon Sep 17 00:00:00 2001 From: "paseo-bot[bot]" <266920839+paseo-bot[bot]@users.noreply.github.com> Date: Wed, 4 Nov 2026 11:09:38 +0000 Subject: [PATCH] fix(app): handle terminal finger scrolling once Keep the vertical gesture in the existing terminal handler so xterm does not also scroll it at document level. Retain only the confirmed propagation correction from the contributor patch and cover the actual phone-sized app with trusted touch input. Co-authored-by: sino --- .../app/e2e/browser/terminal-find.spec.ts | 81 ++++++++++++++ .../app/src/components/terminal-emulator.tsx | 3 +- .../terminal-emulator-runtime.browser.test.ts | 103 ------------------ .../runtime/terminal-emulator-runtime.ts | 48 ++------ 4 files changed, 91 insertions(+), 144 deletions(-) diff --git a/packages/app/e2e/browser/terminal-find.spec.ts b/packages/app/e2e/browser/terminal-find.spec.ts index 91e8e5826a..606b26f753 100644 --- a/packages/app/e2e/browser/terminal-find.spec.ts +++ b/packages/app/e2e/browser/terminal-find.spec.ts @@ -1,3 +1,4 @@ +import { getServerId } from "../support/helpers/server-id"; import { runWorkspaceActionFromCommandCenter } from "../support/helpers/command-center-workspace-actions"; import { spawnSync } from "node:child_process"; import { readFile, writeFile } from "node:fs/promises"; @@ -487,3 +488,83 @@ test("keeps the shared Find controls usable in a compact dark browser", async ({ await page.getByRole("button", { name: "Close Find" }).click(); await expect(input(page)).toBeFocused(); }); + +// A phone browser installs xterm's document-level touch gesture listener too. +// Keep this journey touch-enabled so it catches duplicate scroll handling. +test.describe("terminal history with a finger", () => { + test.use({ isMobile: true, hasTouch: true, viewport: { width: 390, height: 844 } }); + + test("scrolls history once per finger drag", async ({ page }) => { + await openPhoneTerminalHistory(page); + await expectSingleFingerScroll(page, 24); + await expectSingleFingerScroll(page, 1); + await expectMouseWheelScrollsHistory(page); + }); +}); + +async function openPhoneTerminalHistory(page: Page) { + await harness.createTerminal({ + name: "Finger scroll history", + command: "bash", + args: [ + "--noprofile", + "--norc", + "-c", + "for i in $(seq 1 150); do echo HISTORY_$i; done; exec sleep 300", + ], + }); + await page.goto("/"); + await page.getByRole("button", { name: "Open menu", exact: true }).click(); + await page.getByTestId(`sidebar-workspace-row-${getServerId()}:${harness.workspaceId}`).click(); + await expect(page.getByRole("textbox", { name: "Terminal input" })).toBeVisible(); + await expect(page.getByTestId("terminal-surface").filter({ visible: true })).toBeVisible(); + await expect.poll(() => getTerminalBufferText(page)).toContain("HISTORY_150"); +} + +async function expectMouseWheelScrollsHistory(page: Page) { + const beforeWheel = (await viewport(page))!; + await page.mouse.move(200, 300); + await page.mouse.wheel(0, -120); + await expect.poll(() => viewport(page)).toBeLessThan(beforeWheel); +} + +async function expectSingleFingerScroll(page: Page, moves: number) { + await test.step(`Drag history with ${moves} touch moves`, async () => { + const screen = page + .getByTestId("terminal-surface") + .filter({ visible: true }) + .locator(".xterm-screen"); + const box = await screen.boundingBox(); + if (!box) throw new Error("Terminal screen is not visible"); + const rows = await page.evaluate(() => window.__paseoTerminal!.rows); + const lineHeight = box.height / rows; + const before = (await viewport(page))!; + const session = await page.context().newCDPSession(page); + try { + const start = { x: box.x + box.width / 2, y: box.y + box.height / 3 }; + await session.send("Input.dispatchTouchEvent", { + type: "touchStart", + touchPoints: [{ ...start, id: 1 }], + }); + for (let step = 1; step <= moves; step++) { + await session.send("Input.dispatchTouchEvent", { + type: "touchMove", + touchPoints: [{ x: start.x, y: start.y + (120 * step) / moves, id: 1 }], + }); + } + // Assert before lifting the finger so xterm's inertial scrolling is excluded. + await expect.poll(async () => before - (await viewport(page))!).toBeGreaterThan(0); + const moved = before - (await viewport(page))!; + await test.info().attach("finger-scroll", { + body: JSON.stringify({ moves, distancePx: 120, lineHeight, rowsMoved: moved }), + contentType: "application/json", + }); + expect(moved, "one gesture must not scroll twice its physical distance").toBeLessThanOrEqual( + Math.ceil(120 / lineHeight), + ); + } finally { + await session.send("Input.dispatchTouchEvent", { type: "touchEnd", touchPoints: [] }); + await session.detach(); + } + }); +} diff --git a/packages/app/src/components/terminal-emulator.tsx b/packages/app/src/components/terminal-emulator.tsx index 05feeb438c..7504843c54 100644 --- a/packages/app/src/components/terminal-emulator.tsx +++ b/packages/app/src/components/terminal-emulator.tsx @@ -22,7 +22,6 @@ import type { ITheme } from "@xterm/xterm"; import type { TerminalInputModeState } from "@getpaseo/protocol/terminal-input-mode"; import type { PendingTerminalModifiers } from "../utils/terminal-keys"; import { - TERMINAL_SURFACE_TOUCH_ACTION, TerminalEmulatorRuntime, type TerminalOutputData, } from "../terminal/runtime/terminal-emulator-runtime"; @@ -578,7 +577,7 @@ export default function TerminalEmulator({ backgroundColor: xtermTheme.background ?? "#0b0b0b", overflow: "hidden", overscrollBehavior: "none", - touchAction: TERMINAL_SURFACE_TOUCH_ACTION, + touchAction: "pan-y", }), [xtermTheme.background], ); diff --git a/packages/app/src/terminal/runtime/terminal-emulator-runtime.browser.test.ts b/packages/app/src/terminal/runtime/terminal-emulator-runtime.browser.test.ts index 0f0187d89c..bf044237b0 100644 --- a/packages/app/src/terminal/runtime/terminal-emulator-runtime.browser.test.ts +++ b/packages/app/src/terminal/runtime/terminal-emulator-runtime.browser.test.ts @@ -223,56 +223,6 @@ function getBrowserTerminal(): BrowserTerminal { return terminal; } -interface ScrollbackTerminal { - rows: number; - buffer: { - active: { - viewportY: number; - baseY: number; - }; - }; -} - -function getScrollbackTerminal(): ScrollbackTerminal { - const terminal = window.__paseoTerminal as ScrollbackTerminal | undefined; - if (!terminal?.buffer?.active) { - throw new Error("Expected xterm scrollback buffer"); - } - return terminal; -} - -function dispatchTouch(input: { - target: HTMLElement; - type: "touchstart" | "touchmove" | "touchend"; - x: number; - y: number; -}): void { - const touch = new Touch({ - identifier: 1, - target: input.target, - clientX: input.x, - clientY: input.y, - pageX: input.x, - pageY: input.y, - screenX: input.x, - screenY: input.y, - radiusX: 1, - radiusY: 1, - rotationAngle: 0, - force: 0.5, - }); - const ended = input.type === "touchend"; - input.target.dispatchEvent( - new TouchEvent(input.type, { - bubbles: true, - cancelable: true, - touches: ended ? [] : [touch], - targetTouches: ended ? [] : [touch], - changedTouches: [touch], - }), - ); -} - interface DispatchTerminalKeyInput { host: HTMLElement; key: string; @@ -376,59 +326,6 @@ describe("terminal emulator runtime in a real browser", () => { expect(window.__paseoTerminal?.options.scrollback).toBe(42_000); }); - it("scrolls terminal history when a finger drags downward", async () => { - await page.viewport(900, 600); - const mounted = createTerminalHost({ width: 720, height: 360, scrollback: 1_000 }); - - await waitFor({ - predicate: () => mounted.sizes.length > 0 && (window.__paseoTerminal?.rows ?? 0) > 0, - }); - await writeLines( - mounted, - Array.from({ length: 80 }, (_, index) => `history-line-${index}`), - ); - - const screen = mounted.host.querySelector(".xterm-screen"); - const viewport = mounted.host.querySelector(".xterm-viewport"); - if (!screen || !viewport) { - throw new Error("Expected xterm screen and viewport"); - } - await waitFor({ - predicate: () => screen.getBoundingClientRect().height > 0, - }); - - expect(mounted.root.style.touchAction).toBe("none"); - expect(screen.style.touchAction).toBe("none"); - expect(viewport.style.touchAction).toBe("none"); - - const terminal = getScrollbackTerminal(); - const before = terminal.buffer.active.viewportY; - expect(before).toBe(terminal.buffer.active.baseY); - - const rect = screen.getBoundingClientRect(); - const lineHeight = rect.height / terminal.rows; - const startX = rect.left + rect.width / 2; - const startY = rect.top + rect.height / 2; - const draggedLines = 4; - dispatchTouch({ target: screen, type: "touchstart", x: startX, y: startY }); - dispatchTouch({ - target: screen, - type: "touchmove", - x: startX, - y: startY + lineHeight * draggedLines, - }); - dispatchTouch({ - target: screen, - type: "touchend", - x: startX, - y: startY + lineHeight * draggedLines, - }); - - const scrolledLines = before - terminal.buffer.active.viewportY; - expect(scrolledLines).toBeGreaterThanOrEqual(draggedLines - 1); - expect(scrolledLines).toBeLessThanOrEqual(draggedLines); - }); - it("does not claim PTY ownership from passive mount refits", async () => { await page.viewport(900, 600); const mounted = createTerminalHost({ width: 720, height: 360 }); diff --git a/packages/app/src/terminal/runtime/terminal-emulator-runtime.ts b/packages/app/src/terminal/runtime/terminal-emulator-runtime.ts index 970baf65a8..9da1930bbc 100644 --- a/packages/app/src/terminal/runtime/terminal-emulator-runtime.ts +++ b/packages/app/src/terminal/runtime/terminal-emulator-runtime.ts @@ -156,11 +156,6 @@ const isAppleHandheld = }); const DEFAULT_TOUCH_SCROLL_LINE_HEIGHT_PX = 18; -// The screen canvas covers the scrollable viewport, and the page itself does -// not scroll. `pan-y` lets the browser claim a vertical drag and then drop it, -// so a finger swipe never reaches the scrollback. `none` keeps the gesture in -// the touch handler below. -export const TERMINAL_SURFACE_TOUCH_ACTION = "none"; const FIT_TIMEOUT_DELAYS_MS = [0, 16, 48, 120, 250, 500, 1_000, 2_000]; const OUTPUT_OPERATION_TIMEOUT_MS = 5_000; const EMPTY_TERMINAL_OUTPUT = new Uint8Array(0); @@ -690,7 +685,6 @@ export class TerminalEmulatorRuntime { root: input.root, }); const restoreViewportStyles = this.applyViewportTouchStyles({ - root: input.root, host: input.host, }); @@ -1393,15 +1387,9 @@ export class TerminalEmulatorRuntime { }; } - private applyViewportTouchStyles(input: { - root: HTMLDivElement; - host: HTMLDivElement; - }): () => void { + private applyViewportTouchStyles(input: { host: HTMLDivElement }): () => void { const viewportElement = input.host.querySelector(".xterm-viewport"); - const screenElement = input.host.querySelector(".xterm-screen"); - const previousRootTouchAction = input.root.style.touchAction; - const previousScreenTouchAction = screenElement?.style.touchAction ?? ""; const previousViewportOverscroll = viewportElement?.style.overscrollBehavior ?? ""; const previousViewportTouchAction = viewportElement?.style.touchAction ?? ""; const previousViewportOverflowY = viewportElement?.style.overflowY ?? ""; @@ -1409,14 +1397,9 @@ export class TerminalEmulatorRuntime { const previousViewportPointerEvents = viewportElement?.style.pointerEvents ?? ""; const previousViewportWebkitOverflowScrolling = viewportElement?.style.getPropertyValue("-webkit-overflow-scrolling") ?? ""; - - input.root.style.touchAction = TERMINAL_SURFACE_TOUCH_ACTION; - if (screenElement) { - screenElement.style.touchAction = TERMINAL_SURFACE_TOUCH_ACTION; - } if (viewportElement) { viewportElement.style.overscrollBehavior = "none"; - viewportElement.style.touchAction = TERMINAL_SURFACE_TOUCH_ACTION; + viewportElement.style.touchAction = "pan-y"; viewportElement.style.overflowY = "auto"; viewportElement.style.overflowX = "hidden"; viewportElement.style.pointerEvents = "auto"; @@ -1424,10 +1407,6 @@ export class TerminalEmulatorRuntime { } return () => { - input.root.style.touchAction = previousRootTouchAction; - if (screenElement) { - screenElement.style.touchAction = previousScreenTouchAction; - } if (viewportElement) { viewportElement.style.overscrollBehavior = previousViewportOverscroll; viewportElement.style.touchAction = previousViewportTouchAction; @@ -1442,25 +1421,17 @@ export class TerminalEmulatorRuntime { }; } - private touchScrollLineHeightPx(input: { host: HTMLDivElement; terminal: Terminal }): number { - const screen = input.host.querySelector(".xterm-screen"); - const rows = input.terminal.rows; - if (screen && rows > 0) { - const height = screen.getBoundingClientRect().height / rows; - if (height > 0) { - return height; - } - } - return DEFAULT_TOUCH_SCROLL_LINE_HEIGHT_PX; - } - private setupTouchScrollHandlers(input: { root: HTMLDivElement; host: HTMLDivElement; terminal: Terminal; }): () => void { let touchScrollRemainderPx = 0; - let touchScrollLineHeightPx = this.touchScrollLineHeightPx(input); + const measuredLineHeight = + input.host.querySelector(".xterm-rows > div")?.getBoundingClientRect().height ?? + 0; + const touchScrollLineHeightPx = + measuredLineHeight > 0 ? measuredLineHeight : DEFAULT_TOUCH_SCROLL_LINE_HEIGHT_PX; const activeTouch = { identifier: -1, @@ -1494,7 +1465,6 @@ export class TerminalEmulatorRuntime { activeTouch.lastY = touch.clientY; activeTouch.mode = null; touchScrollRemainderPx = 0; - touchScrollLineHeightPx = this.touchScrollLineHeightPx(input); }; const touchMoveHandler = (event: TouchEvent) => { @@ -1534,8 +1504,8 @@ export class TerminalEmulatorRuntime { touchScrollRemainderPx -= lineDelta * touchScrollLineHeightPx; } - // xterm also turns this touch into a scroll on document. Handling it - // twice jumps about two lines per cell. + // xterm also handles touchmove on document. A claimed vertical drag + // must not scroll through both handlers. event.preventDefault(); event.stopPropagation(); };