test(player): assert the desktop bar's size by mechanism, not by pixels
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m24s
CI / e2e (pull_request) Successful in 8m56s

WebKit draws the same button 36x24 where Chromium draws 33x21, so the
literal this pinned failed in CI on a build where nothing was wrong. A
button's box comes from the UA stylesheet when the author sets nothing,
and what each UA sets is its own business.

What must not happen is that *we* set something. So: `min-width` and
`min-height` compute to 0px, the font-size still equals that of a bare
button probed in the same page, and all five boxes are identical --
which is what says the desktop is neither sized context. Checked by
re-introducing the `font-size: inherit` regression, which it catches in
Chromium; the literal form could only be checked by hand.
This commit is contained in:
2026-08-21 00:02:27 -04:00
parent c7e5a4f086
commit e3b64f9255
2 changed files with 67 additions and 14 deletions
+17 -3
View File
@@ -4510,9 +4510,23 @@ stylesheet gives it one, so `font-size: inherit` on a button is a
*change*, not a no-op: it took every desktop control from 33x21 to
36x24 by moving them from 13.3px to the shell's 16px. Nothing failed.
The only way it surfaced was measuring the baseline by stashing the file
and re-running — which is now what `phone-transport.spec.ts` pins, with
a literal `'33x21'` rather than a range, because a range swallows three
pixels.
and re-running.
**And the pixel it was first pinned with was the wrong assertion.** The
spec asserted the literal `'33x21'`, measured in Chromium — and WebKit
draws the same button **36x24**, so it failed in CI on a build where
nothing was wrong. A button's box comes from the UA stylesheet when the
author sets nothing, and what each UA sets is its own business. What
must not happen is that *we* set something, so that is what it asserts
now: `min-width` and `min-height` compute to `0px`, and the font-size
still equals that of a bare `<button>` probed in the same page. That
form catches the `font-size: inherit` regression in either engine —
checked by re-introducing it — and it is the same "assert the
mechanism" move `queue-as-a-screen.spec.ts` makes about containment.
It is also the second time in two sessions that **CI's WebKit was the
only tier that could see something**, which is the argument for checking
that step ran rather than trusting the run's conclusion.
**A rule at the bottom of `index.css` still loses to a nested rule
above it.** The phone block is last on purpose because a media query
+50 -11
View File
@@ -258,19 +258,58 @@ test.describe('the desktop bar is untouched', () => {
await expect(app.locator('#queue-button')).toBeVisible();
});
test('keeps them exactly the size they were', async ({ app }) => {
const sizes = await barControls(app).evaluate((el) =>
[...el.shadowRoot!.querySelectorAll('button')].map((b) => {
/**
* **The mechanism, because the pixels are the engine's.**
*
* The first version of this asserted the literal `'33x21'`, measured
* on `main` in Chromium — and WebKit draws the same button **36x24**,
* so it failed in CI on a build where nothing was wrong. A button's
* box comes from the UA stylesheet when the author sets nothing, and
* what each UA sets is its own business.
*
* What this PR must not do is *set* anything here, so that is what is
* asserted: our two box properties are unset, and the font is still
* the UA's rather than the shell's. That is precisely the regression
* this caught the first time — a generic `font-size: inherit` took
* these from the UA's default to 16px — and it catches it in either
* engine.
*/
test('sets no size of its own on the desktop bar', async ({ app }) => {
const measured = await barControls(app).evaluate((el) => {
// A bare button with no author styles: whatever this engine
// gives one is what the bar's buttons must still be.
const probe = document.createElement('button');
document.body.appendChild(probe);
const uaFontSize = getComputedStyle(probe).fontSize;
probe.remove();
return [...el.shadowRoot!.querySelectorAll('button')].map((b) => {
const cs = getComputedStyle(b);
const r = b.getBoundingClientRect();
return `${Math.round(r.width)}x${Math.round(r.height)}`;
}),
);
return {
minWidth: cs.minWidth,
minHeight: cs.minHeight,
usesUaFont: cs.fontSize === uaFontSize,
size: `${Math.round(r.width)}x${Math.round(r.height)}`,
};
});
});
// Measured on `main` before this change, at 1280x800 and at 424x439
// alike. Written down as a literal rather than as "not bigger",
// because the regression was three pixels and a range would have
// swallowed it.
expect(sizes).toEqual(['33x21', '33x21', '33x21', '33x21', '33x21']);
expect(measured).toHaveLength(5);
for (const m of measured) {
expect(m.minWidth, 'min-width').toBe('0px');
expect(m.minHeight, 'min-height').toBe('0px');
expect(m.usesUaFont, 'font-size is still the UA default').toBe(true);
}
// And all five are the same box: `.play` takes a larger size in
// both sized contexts, so this is what says the desktop is neither
// of them.
expect(new Set(measured.map((m) => m.size)).size).toBe(1);
});
});