From f616a58fb7ac6b00dae59bcd7a811edbbb0b9016 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Tue, 23 Jun 2026 15:52:34 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=96=B1=EF=B8=8F=20fix:=20Summon=20Quote?= =?UTF-8?q?=20Popup=20on=20Double-Click=20Word=20Selection=20(#13923)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🖱️ fix: Summon Quote Popup on Double-Click Word Selection Chromium commits a double-click word selection on the `dblclick` event, after `mouseup` has already read a still-collapsed range, so the "Add to chat" popup never appeared for double-click selections. Listen for `dblclick` in addition to `mouseup`/`keyup`. Adds an e2e covering a native double-click word selection (measured-coordinate dblclick exercises the real browser path, unlike the programmatic-Range helper). * 🎯 test: Target Reply Text Node in Double-Click Quote E2E Walk to the text node containing the needle (not the first text node in .message-render, which may be a select-none screen-reader/model-label header) and measure the needle's first character, so the native double-click lands on the reply word rather than metadata. --- .../src/components/Chat/Input/QuoteButton.tsx | 4 ++ e2e/specs/mock/quotes.spec.ts | 60 +++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/client/src/components/Chat/Input/QuoteButton.tsx b/client/src/components/Chat/Input/QuoteButton.tsx index e2caef8a4a..073d96aeeb 100644 --- a/client/src/components/Chat/Input/QuoteButton.tsx +++ b/client/src/components/Chat/Input/QuoteButton.tsx @@ -92,12 +92,16 @@ function QuoteButton({ conversationId }: { conversationId: string }) { const clearSelection = () => setSelection(null); document.addEventListener('mouseup', updateSelection); + /** Chromium commits a double-click word selection on `dblclick`, after + * `mouseup` has already read a still-collapsed range, so listen here too. */ + document.addEventListener('dblclick', updateSelection); document.addEventListener('keyup', updateSelection); document.addEventListener('scroll', clearSelection, true); window.addEventListener('resize', clearSelection); return () => { document.removeEventListener('mouseup', updateSelection); + document.removeEventListener('dblclick', updateSelection); document.removeEventListener('keyup', updateSelection); document.removeEventListener('scroll', clearSelection, true); window.removeEventListener('resize', clearSelection); diff --git a/e2e/specs/mock/quotes.spec.ts b/e2e/specs/mock/quotes.spec.ts index 25ffe34d60..7271ae8e54 100644 --- a/e2e/specs/mock/quotes.spec.ts +++ b/e2e/specs/mock/quotes.spec.ts @@ -47,6 +47,40 @@ async function selectMessageText(page: Page, needle: string) { }, needle); } +/** + * Double-click the first word of `needle` inside the most recent message + * containing it, using native mouse events at that word's measured coordinates. + * Unlike `selectMessageText` (a programmatic Range), this exercises the + * browser's own double-click word selection — the path the `dblclick` listener + * guards. Measuring the `needle` text node itself (not the first text node in + * `.message-render`, which may be a `select-none` screen-reader/model-label + * header) keeps the click on the actual reply word, not metadata or whitespace. + */ +async function doubleClickWord(page: Page, needle: string) { + const point = await page.evaluate((text) => { + const renders = Array.from(document.querySelectorAll('.message-render')); + const host = [...renders].reverse().find((el) => (el.textContent ?? '').includes(text)); + if (!host) { + throw new Error(`No message contains: ${text}`); + } + const walker = document.createTreeWalker(host, NodeFilter.SHOW_TEXT); + let node = walker.nextNode(); + while (node && !(node.nodeValue ?? '').includes(text)) { + node = walker.nextNode(); + } + if (!node) { + throw new Error(`No text node contains: ${text}`); + } + const index = (node.nodeValue ?? '').indexOf(text); + const range = document.createRange(); + range.setStart(node, index); + range.setEnd(node, index + 1); + const r = range.getBoundingClientRect(); + return { x: r.x + r.width / 2, y: r.y + r.height / 2 }; + }, needle); + await page.mouse.dblclick(point.x, point.y); +} + const addToChat = (page: Page) => page.getByTestId('add-to-chat-button'); const pendingChips = (page: Page) => page.getByTestId('pending-quote-chips'); const messageQuotes = (page: Page) => messagesView(page).getByTestId('message-quotes'); @@ -108,6 +142,32 @@ test.describe('quote references', () => { await expect(messageQuotes(page)).toContainText(MOCK_REPLY_TEXT); }); + test('summons the popup from a native double-click word selection', async ({ page }) => { + test.setTimeout(120000); + await page.goto(NEW_CHAT_PATH, { timeout: 10000 }); + await selectMockEndpoint(page, MOCK_ENDPOINTS[0]); + + const response = await sendMessage(page, 'seed for dblclick'); + expect(response.ok()).toBeTruthy(); + await expect(mockReply(page)).toBeVisible({ timeout: 20000 }); + + // A real double-click selects the word under the cursor. Chromium commits + // that selection on `dblclick`, AFTER `mouseup` fires, so only a `dblclick` + // listener catches it — a programmatic Range (the other tests) would bypass + // this path entirely. Retried as a unit: auto-scroll can clear a fresh + // selection, which races the scripted double-click. + await expect(async () => { + await doubleClickWord(page, MOCK_REPLY_TEXT); + const button = addToChat(page); + await expect(button).toBeVisible({ timeout: 3000 }); + await button.click(); + await expect(pendingChips(page)).toHaveAttribute('data-quote-count', '1'); + }).toPass({ timeout: 30000 }); + + // The quoted excerpt is a word from the reply, not empty. + await expect(pendingChips(page)).toContainText(/E2E|mock|reply/i); + }); + test('collapses multiple selections into one chip with a hover popup, and removes one', async ({ page, }) => {