mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-08-06 15:25:34 +00:00
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 <shaojin.wensj@alibaba-inc.com>
This commit is contained in:
parent
8566385a66
commit
e7362d5d08
3 changed files with 78 additions and 0 deletions
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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 ({
|
||||
|
|
|
|||
|
|
@ -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%);
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue