Lay out and paint form controls — they had no box at all until now - #95
Merged
Merged
Conversation
Writing the F6 capstone test (a login flow driven by pixel coordinates only, the way any real host — go-aiquota/tray's onboarding window, browserproxy's click forwarding — actually drives one, never a direct *dom.Node reference) failed immediately: #email/#password/#submit were missing from the element hit-map entirely. Root cause traced to layout/layout.go's isReplacedTag, which only recognizes "img"/"svg" — input/button/select/textarea default to display:inline (css/ua.go) and have NEVER been given a box or a paint step anywhere in this engine. This is not specific to the F0-F5 work (#89-93): it means an <input> has been invisible and unclickable in this engine, full stop, independent of the JS/event mechanics those PRs correctly built. Two-part fix, since replaced/atomic inline content actually has TWO entry points in this codebase and only one was obvious from reading isReplacedTag's own doc comment: - contents() (layout.go): the rarer path — a control given `display:block` (or similar) by author CSS routes through the normal block-box path. - appendElementInline(): the COMMON path — an inline-display control (the default) nested in ordinary flow content, discovered by instrumenting the box tree directly when the "obvious" fix (only touching contents()) still left the capstone test failing with an empty box tree. Both reuse the same formControlSize/formControlDefaultSize (explicit CSS width/height first, else a UA-shaped default per control kind — checkbox/radio a small square, a button-like control sized to fit its own label via the same Measurer already threaded through layout, everything else a fixed text-field-shaped box) and controlLabel helpers, proven to agree by TestFormControlDisplayBlockRoutesThrough Contents. paint/paint.go: it.FormControl (a new InlineItem field, the Image field's sibling) gets a real background+1px-border box, checkbox/radio a state-colored square, and — for anything else — its value/label text (password masked as bullets, a placeholder drawn muted, a button's label centered, matching real UA behavior close enough to read as intentional rather than exact chrome fidelity). 100% coverage on every new function in both packages; full existing suite + -race unaffected (the one existing fixture using <input>, checkbox_hack.html, sets it display:none via its own CSS, so it never reaches either new code path). The capstone test (e2e_login_test.go) now passes: Open -> Elements()/ElementAt (coordinate resolution) -> Focus/Type/Click, entirely by pixel coordinate, correctly authenticates against a page whose own JS tracks the typed values. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CI's coverage-gate.sh enforces a strict, ratcheted 100% floor on layout (and paint, css, dom) — a floor my local check didn't reproduce, since I'd been comparing against the unratcheted `engine` package's own looser tolerance. Two fixes: - formControlDefaultSize: restructured to named returns assigned by each case and returned once at the bottom, instead of each case returning directly — the trailing fallback after the switch was unreachable by construction (the only two call sites already gate on isFormControlTag), and 100% coverage can't be reached by testing around an unreachable line, only by removing it. - Added tests for three real branches that were reachable but untested: a form control preceded by inline text with trailing whitespace (needs its own SpaceBefore, the same collapsible-space handling img/svg items already get — a real gap, "Label <input>" is the normal shape of an actual form), and the hidden-input check at BOTH its entry points (appendElementInline for the common inline case, contents() for the rarer display:block one) rather than just one. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Writing the F6 capstone test (a login flow driven by pixel coordinates only — the way a real host, e.g. go-aiquota/tray's onboarding window, actually drives one, never a direct
*dom.Nodereference) failed immediately:#email/#password/#submitwere missing from the element hit-map entirely.Root cause:
layout.isReplacedTagonly recognizes"img"/"svg".input/button/select/textareadefault todisplay:inline(css/ua.go) and have never been given a box or a paint step anywhere in this engine. This isn't specific to the F0–F5 interactive-input work (#89–93) — it means an<input>has been invisible and unclickable in this engine, full stop, independently of whether the JS/event mechanics work (they do, correctly).Two-part layout fix, since replaced/atomic inline content has two entry points and only one was obvious from
isReplacedTag's own doc comment:contents()— the rarer path: a control givendisplay:blockby author CSS.appendElementInline()— the common path: an inline-display control (the default) nested in ordinary flow content. Found by instrumenting the box tree directly when the "obvious" fix (onlycontents()) still left an empty tree.Both reuse the same sizing helpers: explicit CSS width/height first, else a UA-shaped default per kind (checkbox/radio a small square, a button-like control sized to fit its own label via the engine's existing text Measurer, everything else a fixed text-field box).
Paint: a new
InlineItem.FormControlfield (sibling ofImage) gets a real background + 1px border, checkbox/radio a state-colored square, and value/label text (password masked as bullets, placeholder drawn muted, button label centered) — close enough to real UA chrome to read as intentional, not pixel-perfect fidelity.Test plan
-racepass unmodified,go vet/gofmtcleanlayoutandpaint<input>(checkbox_hack.html) sets itdisplay:nonevia its own CSS, so it never reaches either new code path — confirmed unaffectede2e_login_test.go'sTestEndToEndLoginByCoordinate— the capstone:Open→Elements()/ElementAt(coordinate resolution) →Focus/Type/Click, entirely by pixel coordinate, correctly authenticates against a page whose own JS tracks the typed values (plus a wrong-password negative control)🤖 Generated with Claude Code