Repository navigation
fix(fill): support date, time, datetime-local, month, and week inputs - #443
Conversation
|
I tested this in Chrome and found two cases that need fixing:
Could you fix these two cases and add browser regression tests? The existing tests pass; these scenarios just aren’t covered yet. |
Address review on Tencent#443: flush deferred background focus before the date-like value write so a later tab switch cannot overwrite the fill, and dispatch composed input/change events so Shadow DOM hosts are notified.
|
Thanks for catching these — both should be fixed in 31cf7b4:
Added unit coverage for both, plus Chrome regressions under |
Chrome's Input.insertText leaves date-like controls empty. Allow those types in the live fill preflight, assign via the input value setter, and reject invalid values the same way as number inputs. Checkbox, radio, range, and file stay out of fill.
Address review on Tencent#443: flush deferred background focus before the date-like value write so a later tab switch cannot overwrite the fill, and dispatch composed input/change events so Shadow DOM hosts are notified.
31cf7b4 to
3539373
Compare
Appending to a date/time value only concatenated strings, so --no-clear either failed with a misleading "invalid value" error or did nothing useful. Reject it before focusing the target and drop the now-dead append handling from the value-write path. Validate date-like values against the input's min/max using the browser's own range validity (which also covers reversed time ranges) and reject out-of-range values before focus with the bounds in the error message. Co-Authored-By: Claude Opus 5.5 <[email protected]>
NianJiuZst
left a comment
There was a problem hiding this comment.
Review: Approve ✅
Thanks @Li-john1021 for the original implementation and for addressing the first round of feedback (deferred-focus flush and composed: true input events). On top of that, four follow-up commits fix the remaining issues found during review. Summary:
1. afdc5a6 — fix(fill): preserve background date input state
- Problem: Clearing a date-like input (
--value "") in a background tab skipped the deferred-focus flush, and an empty expected value was never written back. A pagefocushandler that ran later (e.g. on tab activation) could restore the old value afterfillhad already reported success. - Fix: The value-write path now flushes deferred background focus for every date-like fill, including clears, and always assigns the expected value, even when it is empty, so a focus handler cannot bring back a value that was just cleared.
- Tests: Unit tests plus Chrome regressions for all five types ("keeps a background
<type>clear after deferred focus and tab activation").
2. 05127ff — fix(vom): expose date inputs as fillable references
- Problem: Chrome exposes these controls with accessibility roles (
Date,DateTime,InputTime) that VOM did not treat as interactive. Date-like inputs therefore never got a@eNref, and agents could only target them with CSS selectors. - Fix:
packages/vom/src/render.tsnow renders date-like<input>elements and makes them referenceable. It skips nodes that have no role, are presentational, or are marked unaddressable. - Tests: VOM render tests per type and role. A Chrome end-to-end suite observes and fills each type by ref in the main document, open shadow root, closed shadow root, and iframe.
3. fce6cad — fix(fill): flush background focus before empty text fills
- Problem: The same deferred-focus race also hit text inputs, textareas, and contenteditable elements. An empty fill sends no
insertText, so nothing flushed pending focus events in a background tab, and a later focus handler could overwrite the cleared field. - Fix: On the insert path, empty fills now check readiness and flush deferred background focus before clearing. If deferred focus moves away from the target, the fill fails with
fill_focus_lostand leaves the field untouched. - Tests: Unit tests and a Chrome regression ("does not clear a background text input when deferred focus moves away").
4. 7500107 — fix(fill): reject --no-clear and enforce min/max for date-like inputs
- Problem:
--no-clearon a date/time input joined the two values as strings (2026-10-14+2026-10-15). The result was always invalid and produced a misleading "invalid value" error.- Values outside the input's
min/maxwere accepted and reported as successful, leaving the control in an:invalidstate.
- Fix:
- Date-like inputs now always replace their value.
--no-clearis rejected withfill_value_invalidbefore the target is focused, and the code that was only reachable through it is removed. - Values are checked against
min/maxusing the browser's own range validity, which also handles reversed time ranges such asmin="22:00" max="02:00". Out-of-range values are rejected before focus, and the error names the bounds. - Skill docs are updated to match.
- Date-like inputs now always replace their value.
- Tests: Chrome regressions for
--no-clearrejection with no side effects, out-of-range rejection for all five types (including the reversed time range), and in-range and inclusive-bound fills.
Verification
- CI is green on
7500107, including the real-Chromefill.browser.test.tssuite (168 tests). - Locally: 3546 extension unit tests pass;
biome check,tsc --noEmit, and the eval corpus validation are clean. - The branch is up to date with
mainand has no conflicts.
The fill flow for date-like inputs is now correct for the foreground, background, Shadow DOM, and iframe cases, and agents can target these inputs by ref. LGTM — approving. Recommend squash merge, since the branch has 8 commits including a merge commit.
Summary
Fixes #440.
bsk fillalready treated every<input>as a cheap candidate, but the live preflight only allowedtext|search|tel|url|email|password|number. Date-like controls were rejected withtarget_not_fillable.Chrome's
Input.insertTextalso leavesdate/time/datetime-local/month/weekempty, so typing alone cannot work even if the preflight were loosened.This change:
FILL_EDITABLE_FUNCTIONHTMLInputElementvalue setter (and still dispatchesinput/change)number(fill_value_invalid)checkbox/radio/range/fileout of fillSKILL.mdHappy to adjust naming or coverage if maintainers prefer a narrower set.
Test plan
vitestfill.test.ts+interaction.test.ts(210 passed)fill.browser.test.tsnative-controls suite withBSK_FILL_CHROME(96 passed, including the five date-like targets and remaining rejects)Input.insertTextstays empty on date-like; value setter + events succeed; invalid strings normalize to""