fix(web): tighten mobile line-copy hit testing

Author: pallyoung

packages/web/src/features/terminal-panel/__tests__/xterm-host.test.tsx

@@ -213,6 +213,7 @@ function stubRowsGeometry(
     hostRect?: { x: number; y: number; width: number; height: number };
     rowsRect: { x: number; y: number; width: number; height: number };
     rowHeight: number;
+    screenRect?: { x: number; y: number; width: number; height: number };
   }
 ) {
   vi.spyOn(host, "getBoundingClientRect").mockReturnValue(
@@ -230,6 +231,13 @@ function stubRowsGeometry(
         height: options.rowHeight,
       });
   });
+
+  if (options.screenRect) {
+    const screenElement = host.querySelector(".xterm-screen");
+    if (screenElement instanceof HTMLDivElement) {
+      screenElement.getBoundingClientRect = () => createMockDomRect(options.screenRect!);
+    }
+  }
 }
 
 const mockTerminal = {
@@ -7723,6 +7731,91 @@ describe("XtermHost", () => {
     vi.useRealTimers();
   });
 
+  it("mobile line copy does not copy when a long press lands in the right gutter outside the xterm screen grid", async () => {
+    vi.useFakeTimers();
+    viewportMocks.viewport = "mobile";
+
+    const originalMatchMedia = window.matchMedia;
+    const writeText = vi.fn().mockResolvedValue(undefined);
+    const vibrate = vi.fn();
+    const store = createStore();
+
+    mockTerminal.cols = 80;
+    mockTerminal.buffer.active.viewportY = 15;
+    setMockBufferLines([[15, "x".repeat(80), false]]);
+
+    window.matchMedia = vi.fn().mockImplementation((query: string) => ({
+      matches: query === "(pointer: coarse)",
+      media: query,
+      onchange: null,
+      addListener: vi.fn(),
+      removeListener: vi.fn(),
+      addEventListener: vi.fn(),
+      removeEventListener: vi.fn(),
+      dispatchEvent: vi.fn(),
+    })) as typeof window.matchMedia;
+
+    Object.defineProperty(navigator, "clipboard", {
+      configurable: true,
+      value: { writeText } satisfies Pick<Clipboard, "writeText">,
+    });
+    Object.defineProperty(navigator, "vibrate", {
+      configurable: true,
+      value: vibrate,
+    });
+
+    store.set(localeAtom, "en");
+    store.set(terminalPreferencesAtom, { copyOnSelect: true });
+    store.set(wsClientAtom, {
+      sendCommand: vi.fn().mockResolvedValue({ ok: true, data: { status: "ok" } }),
+      subscribe: vi.fn(() => () => {}),
+      getStatus: vi.fn(() => "connected"),
+      onStatus: vi.fn(() => () => {}),
+      sendTerminalInput: vi.fn().mockResolvedValue(undefined),
+    } as never);
+
+    const { container } = render(
+      <Provider store={store}>
+        <XtermHost
+          terminalId="mobile-line-copy-right-gutter-terminal"
+          workspaceId="test-workspace"
+        />
+      </Provider>
+    );
+
+    const host = container.querySelector(".xterm-host") as HTMLDivElement | null;
+    expect(host).toBeTruthy();
+
+    const screenElement = document.createElement("div");
+    screenElement.className = "xterm-screen";
+    host!.appendChild(screenElement);
+
+    const rowsElement = document.createElement("div");
+    rowsElement.className = "xterm-rows";
+    rowsElement.innerHTML = `<div><span>${"x".repeat(80)}</span></div>`;
+    host!.appendChild(rowsElement);
+    stubRowsGeometry(host!, rowsElement, [rowsElement.firstElementChild as HTMLDivElement], {
+      rowsRect: { x: 0, y: 100, width: 320, height: 20 },
+      screenRect: { x: 0, y: 100, width: 280, height: 20 },
+      rowHeight: 20,
+    });
+
+    dispatchTouchEvent(host!, "touchstart", [{ identifier: 1, clientX: 300, clientY: 110 }]);
+
+    await act(async () => {
+      vi.advanceTimersByTime(500);
+      await Promise.resolve();
+      await Promise.resolve();
+    });
+
+    expect(writeText).not.toHaveBeenCalled();
+    expect(vibrate).not.toHaveBeenCalled();
+    expect(store.get(toastsAtom)).toEqual([]);
+
+    window.matchMedia = originalMatchMedia;
+    vi.useRealTimers();
+  });
+
   it("mobile line copy still succeeds after the row DOM is replaced before long press matures", async () => {
     vi.useFakeTimers();
     viewportMocks.viewport = "mobile";

packages/web/src/features/terminal-panel/mobile/long-press-copy-line.test.ts

@@ -8,11 +8,15 @@ interface MockBufferLine {
   translateToString(trimRight?: boolean): string;
 }
 
-function createBufferLine(text: string, isWrapped = false): MockBufferLine {
+function createBufferLine(
+  text: string,
+  isWrapped = false,
+  noBgTrimmedLength = text.replace(/\s+$/u, "").length
+): MockBufferLine {
   return {
     isWrapped,
     getNoBgTrimmedLength() {
-      return text.replace(/\s+$/u, "").length;
+      return noBgTrimmedLength;
     },
     translateToString(trimRight = false) {
       return trimRight ? text.replace(/\s+$/u, "") : text;
@@ -426,6 +430,105 @@ describe("getLogicalLineTextFromTouchPoint", () => {
     rows.remove();
   });
 
+  it("returns null when the touch point lands in bg-only trailing cells with no copyable text", () => {
+    const { rows, secondRow } = createRowsDom();
+    document.body.appendChild(rows);
+    rows.getBoundingClientRect = () =>
+      ({
+        x: 20,
+        y: 100,
+        top: 100,
+        left: 20,
+        width: 320,
+        height: 60,
+        right: 340,
+        bottom: 160,
+        toJSON: () => ({}),
+      }) as DOMRect;
+    secondRow.getBoundingClientRect = () =>
+      ({
+        x: 20,
+        y: 120,
+        top: 120,
+        left: 20,
+        width: 320,
+        height: 20,
+        right: 340,
+        bottom: 140,
+        toJSON: () => ({}),
+      }) as DOMRect;
+
+    const terminal = createTerminal(16, 90, [[91, createBufferLine("status", false, 10)]]);
+
+    expect(
+      getLogicalLineTextFromTouchPoint({
+        clientX: 185,
+        clientY: 130,
+        rowsElement: rows,
+        terminal,
+      })
+    ).toBeNull();
+
+    rows.remove();
+  });
+
+  it("returns null when the touch point lands in the right gutter outside the screen cell grid", () => {
+    const { rows, secondRow } = createRowsDom();
+    const screenElement = document.createElement("div");
+    document.body.append(rows, screenElement);
+    rows.getBoundingClientRect = () =>
+      ({
+        x: 20,
+        y: 100,
+        top: 100,
+        left: 20,
+        width: 320,
+        height: 60,
+        right: 340,
+        bottom: 160,
+        toJSON: () => ({}),
+      }) as DOMRect;
+    screenElement.getBoundingClientRect = () =>
+      ({
+        x: 20,
+        y: 100,
+        top: 100,
+        left: 20,
+        width: 280,
+        height: 60,
+        right: 300,
+        bottom: 160,
+        toJSON: () => ({}),
+      }) as DOMRect;
+    secondRow.getBoundingClientRect = () =>
+      ({
+        x: 20,
+        y: 120,
+        top: 120,
+        left: 20,
+        width: 320,
+        height: 20,
+        right: 340,
+        bottom: 140,
+        toJSON: () => ({}),
+      }) as DOMRect;
+
+    const terminal = createTerminal(16, 100, [[101, createBufferLine("1234567890abcdef")]]);
+
+    expect(
+      getLogicalLineTextFromTouchPoint({
+        clientX: 310,
+        clientY: 130,
+        rowsElement: rows,
+        screenElement,
+        terminal,
+      })
+    ).toBeNull();
+
+    rows.remove();
+    screenElement.remove();
+  });
+
   it("returns null when terminal columns are unavailable for horizontal hit testing", () => {
     const { rows, secondRow } = createRowsDom();
     document.body.appendChild(rows);

packages/web/src/features/terminal-panel/mobile/long-press-copy-line.ts

@@ -1,6 +1,5 @@
 interface BufferLineLike {
   isWrapped?: boolean;
-  getNoBgTrimmedLength?(): number;
   translateToString(trimRight?: boolean): string;
 }
 
@@ -20,14 +19,11 @@ export interface GetLogicalLineTextFromTouchPointArgs {
   clientX: number;
   clientY: number;
   rowsElement: HTMLElement;
+  screenElement?: HTMLElement;
   terminal: TerminalLikeForLongPressCopy;
 }
 
 function getVisualRowContentLength(line: BufferLineLike): number {
-  if (typeof line.getNoBgTrimmedLength === "function") {
-    return line.getNoBgTrimmedLength();
-  }
-
   return line.translateToString(true).length;
 }
 
@@ -50,11 +46,14 @@ function getVisualRowIndexFromTouchPoint(rowsElement: HTMLElement, clientY: numb
 
 function getBufferRowFromTouchPoint(
   rowsElement: HTMLElement,
+  screenElement: HTMLElement | undefined,
   terminal: TerminalLikeForLongPressCopy,
   clientX: number,
   clientY: number
 ): number | null {
   const rowsRect = rowsElement.getBoundingClientRect();
+  const horizontalBoundsElement = screenElement ?? rowsElement;
+  const horizontalRect = horizontalBoundsElement.getBoundingClientRect();
   if (
     clientX < rowsRect.left ||
     clientX > rowsRect.right ||
@@ -75,7 +74,7 @@ function getBufferRowFromTouchPoint(
     return null;
   }
 
-  if (!Number.isFinite(terminal.cols) || terminal.cols <= 0 || rowsRect.width <= 0) {
+  if (!Number.isFinite(terminal.cols) || terminal.cols <= 0 || horizontalRect.width <= 0) {
     return null;
   }
 
@@ -84,12 +83,16 @@ function getBufferRowFromTouchPoint(
     return bufferRow;
   }
 
-  const cellWidth = rowsRect.width / terminal.cols;
+  if (clientX < horizontalRect.left || clientX > horizontalRect.right) {
+    return null;
+  }
+
+  const cellWidth = horizontalRect.width / terminal.cols;
   if (cellWidth <= 0) {
     return null;
   }
 
-  const columnIndex = Math.floor((clientX - rowsRect.left) / cellWidth);
+  const columnIndex = Math.floor((clientX - horizontalRect.left) / cellWidth);
   if (columnIndex < 0 || columnIndex >= terminal.cols) {
     return null;
   }
@@ -148,6 +151,7 @@ export function getLogicalLineTextFromTouchPoint(
 ): string | null {
   const bufferRow = getBufferRowFromTouchPoint(
     args.rowsElement,
+    args.screenElement,
     args.terminal,
     args.clientX,
     args.clientY

packages/web/src/features/terminal-panel/views/shared/xterm-host.tsx

@@ -715,12 +715,14 @@ export function XtermHost({
         longPressStartClientY = touch.clientY;
         const terminal = terminalRef.current;
         const rowsElement = container.querySelector(".xterm-rows");
+        const screenElement = container.querySelector(".xterm-screen");
         longPressLineText =
           terminal && rowsElement instanceof HTMLElement
             ? getLogicalLineTextFromTouchPoint({
                 clientX: touch.clientX,
                 clientY: touch.clientY,
                 rowsElement,
+                screenElement: screenElement instanceof HTMLElement ? screenElement : undefined,
                 terminal,
               })
             : null;