diff --git a/packages/diffs/src/editor/editor.css b/packages/diffs/src/editor/editor.css index a65a8bf78..c50364d95 100644 --- a/packages/diffs/src/editor/editor.css +++ b/packages/diffs/src/editor/editor.css @@ -275,8 +275,14 @@ border: 0; pointer-events: auto; + /* `contrast-color()` returns black or white against the severity fill. A + theme can set that fill through `editorWarning.foreground` and the similar + tokens, so a fixed text color can fail. + Pass one argument only. Two arguments make the declaration invalid at + computed-value time: `color` then inherits the editor foreground and + ignores the `--diffs-marker-contrast` declaration above. */ @supports (color: contrast-color(red)) { - color: contrast-color(var(--diffs-marker-bg), var(--diffs-bg)); + color: contrast-color(var(--diffs-marker-bg)); } [data-marker-message] { diff --git a/packages/diffs/test/cssContrastColorArity.test.ts b/packages/diffs/test/cssContrastColorArity.test.ts new file mode 100644 index 000000000..e42eb2993 --- /dev/null +++ b/packages/diffs/test/cssContrastColorArity.test.ts @@ -0,0 +1,83 @@ +import { describe, expect, test } from 'bun:test'; +import { readFileSync } from 'fs'; +import { resolve } from 'path'; + +// CSS files that ship with the package. The tests below check every +// `contrast-color()` call in them. +const STYLESHEETS = ['../src/style.css', '../src/editor/editor.css']; + +// Splits a `contrast-color()` argument list into its top-level arguments. A +// comma inside `var()`, `light-dark()`, or `color-mix()` does not split. +function splitTopLevelArgs(args: string): string[] { + const parts: string[] = []; + let depth = 0; + let current = ''; + for (const char of args) { + if (char === '(') { + depth += 1; + } else if (char === ')') { + depth -= 1; + } else if (char === ',' && depth === 0) { + parts.push(current.trim()); + current = ''; + continue; + } + current += char; + } + parts.push(current.trim()); + return parts; +} + +// Returns the argument list of every `contrast-color()` call in `css`. It counts +// nested parens to find the closing paren of each call. +function findContrastColorArgs(css: string): string[] { + const calls: string[] = []; + const marker = 'contrast-color('; + let index = css.indexOf(marker); + while (index !== -1) { + let depth = 1; + let cursor = index + marker.length; + while (cursor < css.length && depth > 0) { + if (css[cursor] === '(') { + depth += 1; + } else if (css[cursor] === ')') { + depth -= 1; + } + cursor += 1; + } + calls.push(css.slice(index + marker.length, cursor - 1)); + index = css.indexOf(marker, cursor); + } + return calls; +} + +// Regression test for white text on the yellow warning popover. +// `contrast-color()` takes one argument. A call with two arguments still parses, +// because `var()` defers the grammar check to substitution. The declaration is +// then invalid at computed-value time, so `color` inherits its value and ignores +// the `--diffs-marker-contrast` declaration. No browser test catches this: +// Playwright bundles an old Chromium without `contrast-color()`, so it runs the +// fallback branch only. +describe('contrast-color() arity in shipped CSS', () => { + for (const stylesheet of STYLESHEETS) { + test(`${stylesheet} passes one argument per call`, () => { + const css = readFileSync(resolve(__dirname, stylesheet), 'utf-8'); + for (const args of findContrastColorArgs(css)) { + expect( + splitTopLevelArgs(args), + `contrast-color(${args}) must take exactly one argument` + ).toHaveLength(1); + } + }); + } + + test('detects a two-argument call', () => { + expect( + splitTopLevelArgs( + findContrastColorArgs( + 'a { color: contrast-color(var(--a), light-dark(#000, #fff)); }' + )[0] + ) + ).toHaveLength(2); + }); +}); diff --git a/packages/diffs/test/e2e/markers.pw.ts b/packages/diffs/test/e2e/markers.pw.ts index 417ba3078..c3f1c7caa 100644 --- a/packages/diffs/test/e2e/markers.pw.ts +++ b/packages/diffs/test/e2e/markers.pw.ts @@ -87,7 +87,9 @@ function openScrolledMarkerNearGutter(page: Page): Promise<{ // contrast ratio between the popover's resolved text and background colors. // Guards the marker popover fallback in browsers without contrast-color(): the // severity fill is a theme editorX.foreground token, so its text candidate must -// remain legible in both light and dark themes. +// remain legible in both light and dark themes. Playwright bundles an old +// Chromium without contrast-color(), so this test measures the fallback branch +// only. cssContrastColorArity.test.ts covers the `@supports` branch. async function popoverContrast( page: Page, token: string