From e7362d5d083de88dfbdee8fa3958703285be9d53 Mon Sep 17 00:00:00 2001 From: C0d3N1nja97342 Date: Tue, 4 Aug 2026 20:33:51 +0800 Subject: [PATCH] fix(web-shell): add explicit ::selection for message content in Firefox (#8417) * fix(web-shell): add explicit ::selection for message content in Firefox Firefox does not paint the default selection highlight for text whose element chain passes through a display:contents element (the data-user-selectable wrapper on MessageItem). The logical selection (copy, selectionchange popup) works fine - only the visual highlight is missing. An explicit ::selection background makes Firefox paint the highlight where the default painting fails. Fixes #8214 * fix: use fixed color instead of non-existent CSS variable --selection-bg was never defined in the codebase (only --chat-editor-selection-bg exists in App.module.css). Use a fixed hsl(210 100% 50% / 30%) to avoid confusion. * test(web-shell): pin ::selection rule and soften root-cause framing Reframe the standalone.css comment and PR description as a defensive workaround, not a confirmed root-cause fix: the data-user-selectable wrapper is shared by user and assistant rows, and the reporter's screenshot shows an embedding-page toolbar this package does not ship. Add a getComputedStyle(..., '::selection') assertion to the smoke e2e so a future cleanup cannot silently drop the rule. * fix(web-shell): move ::selection rule to component-scoped globals.css The defensive ::selection rule for [data-user-selectable] message content was in standalone.css, which is only loaded by the standalone app entry (client/main.tsx) and the e2e harness. The npm package entry (client/index.tsx via vite.lib.config.ts) never loads standalone.css, so embedded deployments of @qwen-code/web-shell - including the reporter of #8214 - did not receive the rule and still saw no selection highlight. Move it to globals.css, which is imported by App.tsx and WebShellTranscript.tsx and therefore ships with the component-scoped stylesheet. Verified against the lib build: the rule now appears in dist/index.js correctly scoped under [data-web-shell-root][data-web-shell-shadcn]. The standalone app also loads globals.css, so the e2e smoke pin still passes. Addresses the review finding on standalone.css:119. * test(web-shell): pin ::selection across all rows and in the lib bundle Address review findings on the round-3 move to globals.css: - The smoke e2e only sampled the first [data-user-selectable] row (the user row in this fixture). Assert the rule on every selectable row so a future narrowing to user rows keeps assistant rows covered. - Nothing asserted the rule survives in the npm lib bundle - the deployment this fix exists for. Add a build-artifact test that parses the injected component CSS in dist/index.js and pins the scoped [data-user-selectable] ::selection rule under [data-web-shell-root]. * test(web-shell): assert ::selection on selectable wrapper rows, not descendants Per review: querySelectorAll('[data-user-selectable] *') counts element descendants, not the wrapper rows themselves - a single user row renders 4+ descendants, so the >=2 invariant did not actually enforce that both roles are present. Match the [data-user-selectable] wrappers directly and sample one descendant per row. * test(web-shell): match ::selection lib-bundle pin by effect, not notation Per review (R6-1): the pin matched an exact selector substring (including the space) and an exact prop name, coupling to the current notation. A maintainer switching 'background' to 'background-color' (the CSS Pseudo-Elements-4 name) would fail this test with a misleading message while the e2e pin stayed green. Match the two selector halves independently and accept either prop name. --------- Co-authored-by: Shaojin Wen --- .../web-shell/client/build-artifact.test.ts | 32 +++++++++++++++++++ .../client/e2e/web-shell.smoke.spec.ts | 24 ++++++++++++++ packages/web-shell/client/styles/globals.css | 22 +++++++++++++ 3 files changed, 78 insertions(+) diff --git a/packages/web-shell/client/build-artifact.test.ts b/packages/web-shell/client/build-artifact.test.ts index 9685e6307e..5d5edebde0 100644 --- a/packages/web-shell/client/build-artifact.test.ts +++ b/packages/web-shell/client/build-artifact.test.ts @@ -217,4 +217,36 @@ describe('build artifact — package boundary', () => { }); expect(unscoped).toEqual([]); }); + + it('ships the ::selection highlight for message content in the lib bundle (#8214)', () => { + // The defensive ::selection rule must reach embedded deployments - + // i.e. it must be in the component-scoped CSS injected into dist/index.js, + // not only the standalone app's standalone.css. Asserting the rule is + // present and scoped under the WebShell root pins the lib-bundle fix. + let matched: Rule | undefined; + postcss.parse(readInjectedCss()).walkRules((rule) => { + // Match the effect (selectable rows get a ::selection rule scoped to + // the WebShell root), not the exact notation - a maintainer changing + // `background` to `background-color` (the CSS Pseudo-Elements-4 name) + // should not break this pin while the e2e one stays green. + if ( + rule.selector.includes('[data-user-selectable]') && + rule.selector.includes('::selection') + ) { + matched = rule; + } + }); + expect( + matched, + '::selection rule for [data-user-selectable] missing from lib bundle', + ).toBeDefined(); + expect(matched?.selector).toContain('[data-web-shell-root]'); + expect( + matched?.nodes.some( + (n) => + n.type === 'decl' && + (n.prop === 'background' || n.prop === 'background-color'), + ), + ).toBe(true); + }); }); diff --git a/packages/web-shell/client/e2e/web-shell.smoke.spec.ts b/packages/web-shell/client/e2e/web-shell.smoke.spec.ts index c564179642..080ccb639c 100644 --- a/packages/web-shell/client/e2e/web-shell.smoke.spec.ts +++ b/packages/web-shell/client/e2e/web-shell.smoke.spec.ts @@ -40,6 +40,30 @@ test('loads replayed transcript and connects to fake daemon @smoke', async ({ await expect(page.locator('[data-web-shell-message-list]')).toContainText( 'Hello from fake daemon', ); + + // #8214: pin the explicit ::selection rule on message content. This + // asserts the rule is present and matches every [data-user-selectable] + // wrapper row (user and assistant alike), not just the first one; it + // does not verify the Firefox paint effect itself (this repo's Playwright + // projects are chromium-only). + const selectionBackgrounds = await page.evaluate(() => { + // Match the wrapper rows themselves, not their descendants - a single + // row renders many descendant elements, so counting descendants does + // not enforce the "both roles present" invariant. + const rows = document.querySelectorAll('[data-user-selectable]'); + return Array.from(rows, (row) => { + // ::selection applies to the element's text content; sample the first + // text-bearing descendant (or the row itself if it has none). + const target = row.querySelector('*') ?? row; + return getComputedStyle(target, '::selection').backgroundColor; + }); + }); + // The fixture renders both a user and an assistant message, so there must + // be at least two selectable rows and every one must carry the rule. + expect(selectionBackgrounds.length).toBeGreaterThanOrEqual(2); + for (const bg of selectionBackgrounds) { + expect(bg).toBe('rgba(0, 128, 255, 0.3)'); + } }); test('submits a prompt and renders a streamed assistant response @smoke', async ({ diff --git a/packages/web-shell/client/styles/globals.css b/packages/web-shell/client/styles/globals.css index 0389bffccc..9c2dd8c824 100644 --- a/packages/web-shell/client/styles/globals.css +++ b/packages/web-shell/client/styles/globals.css @@ -132,3 +132,25 @@ *::-webkit-scrollbar-thumb:hover { background: var(--scrollbar-thumb-hover); } + +/* + * Defensive selection-highlight rule for message content. #8214. + * + * Firefox is suspected of not painting the default selection highlight for + * text whose element chain passes through a `display: contents` element (the + * `data-user-selectable` wrapper on MessageItem). The logical selection + * (copy, selectionchange popup) works fine - only the visual highlight is + * missing. An explicit `::selection` background makes Firefox paint the + * highlight where the default painting may fail. This is a low-risk + * workaround, not a confirmed root-cause fix: the wrapper is shared by user + * and assistant rows alike, and the reporter's environment appears to be an + * embedding page, so its own CSS may also need adjustment. + * + * This rule lives in the component-scoped stylesheet (loaded by App.tsx and + * WebShellTranscript.tsx) rather than standalone.css so it ships with the + * npm package and applies to embedded deployments, not just the standalone + * app. + */ +[data-user-selectable] ::selection { + background: hsl(210 100% 50% / 30%); +}