fix(element): assert the placeholder paints, not what getComputedStyle says - #256
Conversation
…e says `placeholder.spec.ts` read the prompt back with `getComputedStyle(el, '::before').content` and looked for the placeholder text in it. CSS Values 4 substitutes `attr()` at used-value time, so Gecko returns the specified value, `attr(data-placeholder)`, while painting the string perfectly well; Blink and WebKit resolve it early and return the quoted text. The poll could never match on Firefox, and every nightly run since the spec landed has been red for a difference no reader would see. The prompt itself is fine on all three engines: Firefox lays out a 128.07px `::before` against Chromium's 127.5px for the same string. So the spec now asserts the box instead of the string. A generated `::before` carrying text has a measured width and an ungenerated one reports `auto`, identically everywhere -- and that is the stronger claim, since `content` naming a string does not prove the engine laid anything out. A longer prompt painting a wider box binds those glyphs to `data-placeholder` on every engine, which is the part the old assertion was really there for. `content: none` after one character stays as it was; that value is portable. Test-only. Verified against chromium, firefox and webkit, and mutation checked: changing the rule to `content: ''` fails all three. Closes #254 Signed-off-by: Peyton Nowlin <peytonn98@googlemail.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c144582a0a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return page.getByRole('textbox', { name: 'Post body' }) | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
Commit ccaaadb068a79e349c9391eb0e910f560caafe66 has no Signed-off-by: trailer, so the repository's Sign-off job will reject the PR because it checks every commit with no exemption for test-only changes. Recreate this commit with a trailer matching the commit author.
AGENTS.md reference: AGENTS.md:L7-L11
Useful? React with 👍 / 👎.
1 participant
Closes #254.
What was red
packages/element/test/e2e/placeholder.spec.tshas failed on Firefox in every nightly since it landed in #235 — six consecutive scheduled runs (2026-08-26 through 2026-08-31). Pull requests runverify --quick, chromium only, so nothing on the PR path could have caught it.Why
The assertion read the prompt's text back out of the pseudo-element:
CSS Values 4 substitutes
attr()at used-value time. Gecko honours that and returns the specified value,attr(data-placeholder); Blink and WebKit resolve it early and hand back the quoted string. The substring could never appear on Firefox however long the poll ran.The prompt was never broken
#254 asked for confirmation before touching anything, since the point of the test is that the prompt is visible. It is: Firefox lays out a 128.07px
::beforeforWrite the article…against Chromium's 127.5px and WebKit's 127.5px, in the same muted colour. Only the read-back differed.What this does
Asserts the box rather than the string, because the box is identical on all three engines:
::beforecarrying text has a measured width; one that was never generated reportsauto. This is also the stronger claim:contentnaming a string does not prove the engine laid anything out.data-placeholder— a longer prompt paints a wider box, same font. That is what the old text assertion was really there for, and it now holds everywhere rather than on two engines out of three.content: noneafter one character, unchanged; that value is portable.The
valueassertions (the prompt must not serialize intovalue) are untouched.Verification
pnpm exec playwright test packages/element/test/e2e/placeholder.spec.ts— passes on chromium, firefox and webkit.PLACEHOLDER_CSStocontent: ''fails all three engines, so the test still has teeth.pnpm verify(all three engines) run locally. The only failure isdemo.spec.ts:439"serves a promo video the browser can actually decode" on webkit — that is demo: the promo video is H.264-only, so a browser without proprietary codecs shows a dead player (and fails e2e on webkit) #255, it reproduces on a clean checkout ofmainwith this branch stashed, and it passes on the CI runners.Documentation
No documentation impact. Product code is untouched and the placeholder's documented behaviour is unchanged —
docs/api-reference.md:39already describes it as painted via::beforeand never stored invalue, which is still exactly right. NoCHANGELOG.mdentry for the same reason: per AGENTS.md the entry is owed by a behaviour change, and this changes only how a test reads.