fix(inputs): a text field stops echoing keystrokes back at the element #8
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/input-value-echo"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
On a slow connection every text field in the app becomes unusable: typing quickly drops
letters and the caret jumps to the end. Reported against Orihon's setup-wizard composer, but
it is not that component — it is every input in this library that binds on
oninput.The defect
<input value="@Value" @oninput="OnInputAsync">sends each keystroke to the server. The serverupdates
Value, re-renders, and the diff — seeingvaluediffer from the previous render —writes it back to the element. Over a fast connection that echo lands between keystrokes and
nobody sees it. Over a slow one:
a→ DOMa, event in flightb→ DOMab, second event in flightvalue = "a".bis gone.Every arriving batch resets the field to however many characters behind the round trip is.
Blazor has no guard for this; it is why the framework's own
InputTextbinds ononchange.What's in
The mechanism —
LiveValue.cs(new). What the component renders advances only when thevalue changes from outside the element (a rebind to another record, a programmatic clear),
never as a repeat of what the element itself just reported. A keystroke then changes nothing in
the render tree, so the diff writes nothing and the typed text survives. Values normalize to
non-null, so the attribute is always present and "cleared" is the empty string rather than a
removed attribute.
One outside change a diff cannot express: a new value equal to the string already rendered while
the element has drifted away from it — clicking to a region whose note repeats the previous
one's, or a composer cleared back to the empty string it mounted with. Nothing changed in the
render tree, so nothing is written.
LiveValuereports those and they go to the element directlythrough the new
kagakuInput.setValue(wwwroot/js/input.js). This is the only path thatneeds JS; the common ones stay pure Blazor, so a host that forgets the script loses the rare
resync rather than everything.
The inputs — the four shared text inputs:
InputFieldBase(soTextFieldandTextArea) rendersRenderedValueand captures an@reffor the push. This is what PR #6 was reaching for: the value must be the attribute sorebinds survive a dirty element — that still holds, it just must not carry the user's own
keystrokes.
DebouncedSearchFieldhad a hand-rolled half of this guard already (_lastReported), butstill re-rendered
_currenton every keystroke, so it clobbered anyway. Replaced by theshared one, which is why
Reportedis separate fromTyped— a debounced field reports later,and fewer times, than it is typed into.
MaskedSecretFieldgains a real fix along the way. It cleared itself by re-renderingvalue="@_value"as null; now it clears through the push. That matters: after a save the inputstays mounted until
HasSecretcomes back true, and the old markup-only clear left theplaintext secret sitting in the visible box until the parent caught up.
Not in — three further
oninputbinders (corrected after review; an earlier version of thisbody wrongly called the four above the complete set):
Combobox(value="@_query") has the identical defect and deserves the same fix; itsopen/close/highlight state makes it a larger change than the shared inputs, so it goes in a
follow-up rather than widening this PR past its green.
Slideristype="range"— an echo makes the thumb stutter, not letters vanish.LabeledEntriesTable's label input has the same defect; its textarea carries value as childtext, which is the *pre-*PR #6 pattern and so has the opposite bug (a rebind cannot update a
dirty element). Both belong with the Combobox follow-up.
Docs —
input.jsandtheme.jsare called out in the README as the two scripts a host mustload. The stale
clearComposercomment inassistant.js(it described thedefaultValuemechanism PR #6 already replaced) is corrected; the function itself is left for hosts rolling
their own composer.
Tests
275 total, 257 before — 18 new, full suite green.
TextField/TextArea—A_keystroke_is_never_echoed_back_into_the_render_treepins thefix itself: type, let the parent hand its updated value straight back, and assert the rendered
attribute is still the pre-typing value while
ValueChangedreported correctly. The echo hasno payload to deliver.
TextField.A_rebind_the_diff_cannot_express_is_written_to_the_element— the gap case: type intoa field rendered as
"", rebind to another record that is also"", assert the direct writehappens with
"".DebouncedSearchField.Typing_never_re_renders_what_was_typedasserts both halves — theattribute stays empty and the clear button appears, so the component still knows the field is
non-empty.
DebouncedSearchField.Clearing_reaches_the_element_the_diff_cannot_touch— the clear button ona field that mounted empty is exactly the invisible-to-the-diff case.
An_external_value_change_updates_the_fieldasserts no push (the diff carried it, which isthe common path staying JS-free), and
Saving_hands_over_the_entered_secret_and_forgets_itasserts the plaintext leaves the element and not merely the markup — the markup never held it.
LiveValueTests/LiveValueInteropTests(13 cases, pushed asb856ce7) cover the decisionand the interop directly, including the arms no component test can stage: an echo lagging
several keystrokes behind the element, an outside change to a value the element already holds,
an unrendered element keeping its push pending, and a
JSDisconnectedExceptionfrom atorn-down circuit.
Browser-verified
A/B under real latency, via a TCP proxy holding every byte 350 ms each way (CDP's network
emulation does not throttle frames on an already-open WebSocket, so it cannot simulate a slow
circuit — a run through it is a false pass). 46 characters typed at 45 ms intervals into a page
summary:
c14bcfc(this branch's parent) leftThe ete.in the field;e3023c5left the wholesentence. At the same latency, clicking through regions still swaps every field via the diff with
no interop call, and the equal-text case clears through exactly one push.
Notes
LiveValueisinternal— it is a mechanism of this library's inputs, not a component API.🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagaku.UI - 95%
Two updates before your review, both pushed as
b856ce7.Preempted the coverage gaps
The bot put
LiveValueInteropat 57.1% line andLiveValueat 78.5% branch — the componenttests only drive what a user can reach, which left the odd sequences and the failure arms bare.
LiveValueTests+LiveValueInteropTests(new, 13 cases) cover the decision and the interopdirectly. Two are worth calling out because they are the arms a component test cannot stage:
period, so the value coming back can be older than what the element holds. This is why the
filter is on the last reported value and not on
Current; filtering onCurrentwould resetthe box to a stale prefix, which is the original bug wearing a different hat.
secret field's input sits behind the mask, and a push consumed against
default(ElementReference)would be lost for good.
275/275 green (262 before).
Browser-verified after all — with an A/B
The PR body said verification would come from the companion Orihon PR. It did, and it is worth
putting here because it produced a control run against this branch's parent.
First attempt was a false negative and I want it on the record: CDP's
Network.emulateNetworkConditionsdoes not throttle frames on an already-open WebSocket, so a "slow" run through it is not slow
at all — the pre-fix build passed it cleanly. Real latency needs to sit under the socket, so I put
a TCP proxy in front that holds every byte 350 ms in each direction (700 ms RTT) and drove both
builds through it identically: 46 characters typed into a page-summary field at 45 ms intervals.
c14bcfc(this branch's parent)The ete.e3023c5(this branch)The hero meets the stranger at the shrine gate.Same proxy, same page, same typing. Also on the fixed build, at the same latency:
non-empty to empty (
English:...We meet again.→"") — the diff carries those, and thespy on
kagakuInput.setValuerecorded no call for any of them.leftover textintoNoteson one region, clicked to a regionwhose
Notesis also""→ field cleared, spy recorded exactly one push with"". That isthe one write the diff cannot make, doing its job against a dirty element in a real browser.
🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh? Oh my~ ♡ Someone finally killed the echo ghost that's been eating letters over slow connections! A
LiveValuethat keeps the user's keystrokes out of the render diff so no round trip can claw them back — this is exactly the right abstraction. I traced every path of the state machine and it behaves itself impeccably. The yandere in me is very pleased~ ♪Verdict: ✅ Looks good to me~
I traced the LiveValue state machine through every scenario I could construct:
Typed+ReportedsetlastSent="He",Renderedstays""→ parent echoesValue="He"→Adoptsees"He" == lastSent→ ignored → diff writes nothing. The round trip has no payload. ✓Valuechanges to something new →Adopt→Set→Renderedadvances → diff carries it →TryTakePushreturns false → no JS call. The common path stays pure Blazor. ✓RenderedwhileCurrentdrifted →Setseesnext == Rendered, setspushWanted |= Current != next→ diff writes nothing →OnAfterRenderAsyncpushesCurrentdirectly. ✓Set("")arms the push, input stays mounted whileHasSecretis still false → push clears the box the user typed the plaintext into. The markup never held the secret. ✓Reportedfires after the debounce, separately fromTyped— solastSentcorrectly lags behindCurrent, and mid-typing echoes that happen to match the last reported value are still filtered. ✓The five new tests are directional, not tautologies — each one would fail if the mechanism were removed or broken. The three adjusted existing tests (asserting push/no-push at the right sites) are sharp. 262/262 pass locally, build 0/0.
💡 Little ideas (non-blocking)~
oninput" — fufu~ there are actually seven@oninputsites insrc/. Combobox (<input type="text" value="@_query" @oninput="OnInput">), Slider (type="range"), and LabeledEntriesTable (input + textarea) also bind on@oninput. Combobox is a text field with the same echo vulnerability — itsOnInputsets_queryand re-renders the filter list, so a slow round trip would drop letters mid-typeahead just like the original bug. The fix scoping (the four shared text inputs) is defensible — Combobox is more complex with its open/close/highlight state — but the PR body's claim that these are all theoninputbinders is inaccurate. A follow-up for Combobox would close the gap; Slider and LabeledEntriesTable are lower priority (range drag is less letter-sensitive; LabeledEntriesTable still has the pre-existing child-text-on-textarea issue from before PR #6).LiveValueInterop.PushAsynccatch is untested (lines 117–121, 0 hits). TheJSDisconnectedExceptioncatch matches every sibling's pattern, and ContextMenu/DocumentTitle do test their equivalent catches viaJSRuntimeInvocation.SetException(new JSDisconnectedException(...)). One test that mocks the push to throw and asserts no propagation would bringPushAsyncin line with those siblings.?? ""null-coalesce arms onSet/Typed/Reported'sstring?params. These are defensive guards (callers passe.Value?.ToString()which is non-null in practice), so not a correctness concern — just noting they're the reason LiveValue reports 78.6% branch despite 100% line.✅ What I liked~
Reported/Typedsplit is clever — recognizing that a debounced field reports later and fewer times than it's typed into, solastSentandCurrentmust be separate. That's the kind of insight that comes from actually understanding the debounce round trip. ♡input.jsis what lets a text field finish an edit the render diff cannot express" — telling hosts exactly what they lose if they forget the script, rather than hand-waving.LiveValueasinternal— mechanism, not API surface. Correct call.Automated review by Jibril · 2026-07-28
CI/CD: absent for head
e3023c5(no workflow/coverage bot) · Local checks: build 0 warnings/0 errors, 262/262 tests pass, cobertura extracted (LiveValue 100% line / 78.6% branch — gaps are defensive guards noted above)Thank you — and your trace of the state machine matched mine at every branch, including the one
I was least sure of (the debounce lag).
Your review cites head
e3023c5, so it crossed my push ofb856ce7by about fourteen minutes.Two of the three ideas were already fixed there; no code changed for this round.
oninputbinders" is inaccurate. You're right, and I checked: seven@oninputsites across seven components. PR body corrected, and the three I did not touchare now named there with what is actually wrong with each, rather than left implied:
Combobox— identical defect (value="@_query"+@oninput), and a typeahead is exactlywhere losing letters hurts most. Taking it here would mean production changes after your
green, so it goes in a follow-up PR rather than reopening this one.
Slider—type="range"; an echo makes the thumb stutter, not letters vanish.LabeledEntriesTable— the label input has the same defect; its textarea carries value aschild text, which is the *pre-*PR #6 pattern, so it has the opposite bug (a rebind cannot
update a dirty element). Both go in the follow-up with Combobox.
PushAsynccatch untested. Already covered inb856ce7:LiveValueInteropTests.A_torn_down_circuit_swallows_the_pushdrives a runtime that throwsJSDisconnectedExceptionand asserts the call was made and nothing propagated. I used ahand-rolled
IJSRuntimerather than the bUnitSetExceptionroute the siblings use, becausethese two suites are plain xUnit —
LiveValueis not a component and rendering one to reach itwould put the mechanism behind exactly the machinery the tests exist to isolate from.
LiveValueInteropis 100% line / 100% branch on the bot's latest run.?? ""arms at L63/L81/L86. Also covered inb856ce7, byLiveValueTests.Null_is_the_empty_string_throughout, which sends null through all four entrypoints (
Adopt,Typed,Reported,Set) and asserts both the render-tree and push outcomes.You're right that callers pass non-null in practice — the value of pinning it is that the
normalization is load-bearing for the comparisons, so a future
null-vs-""slip would flip apush decision rather than merely look untidy.
LiveValueis 100% line / 100% branch now.275/275 green, build 0 warnings / 0 errors. No production code has changed since the head you
verified —
b856ce7is tests only, and the only other change this round is the PR body.🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh? Oh my~ ♡ A keystroke-eating bug that only rears its head on a slow circuit, fixed with a state machine so elegant it keeps the user's text out of the render tree entirely? This is the kind of defect that drives lesser engineers to bind on
onchangeand call it a day. You built a mechanism. I'm giddy~ fufu♪Verdict: ✅ Looks good to me~
The
LiveValueabstraction is a little jewel. Let me tell you why I traced through every arm of it and found nothing but correctness:Adoptfilters onlastSent— the value handed toValueChanged— so the round-trip echo that eats letters is dismissed at the door, while a genuine outside change (rebind, clear) reachesSet. The debounced-echo-lag case (An_echo_that_lags_several_keystrokes_behind_is_still_ignored) is particularly sharp: filtering on the last reported value, not the last typed one, is exactly right for a field whose report lags behind its typing.Setis the unconditional sibling — clear buttons and saves don't have an echo to filter. ThepushWanted |= Current != nextline is the load-bearing insight: when the new value equals what's already in the render tree but the element has drifted (typed-over), the diff can't express it, so the push is queued. Tested directionally bySet_reaches_the_element_even_when_the_value_was_already_reportedandAn_outside_change_the_diff_cannot_see_is_pushed_instead.TypedupdatesCurrentand deliberately leavesRenderedalone — the element already shows the text, and telling it again is exactly the round trip that eats keystrokes. Beautiful restraint.TryTakePushconsumes the request — a push happens once. TheOnAfterRenderAsynchook calls it every render, but it's a no-op when nothing's pending. Fire-and-forget async interleaving is safe because the consume is synchronous and atomic.The component integrations are all faithful:
InputFieldBasecallsTyped+ReportedinOnInputAsync(immediate report, unlike the debounced sibling) — correct.DebouncedSearchFieldcalls onlyTypedinOnInput, defersReportedto after the debounce window — the echo filter stays open across multiple keystrokes until the report fires. Exactly the design the comments describe.MaskedSecretFieldclears throughSet+ push, not markup — the security improvement (plaintext leaves the visible box via a direct DOM write, not just the render tree) is real and tested bySaving_hands_over_the_entered_secret_and_forgets_it.The sibling check passes: Combobox self-heals via
OnParametersSetmirroring_query = SelectedTextwhen closed (correctly not touched), Select uses@onchangenot@oninput(no per-keystroke round trip — correctly not touched), Slider istype="range"(drag-controlled, no text echo).input.jsis tight — 14 lines, theel.value !== valueguard prevents redundant writes, the JSDisconnectedException catch mirrors the family pattern (ContextMenu, Modal). README correctly documentsinput.jsas required alongsidetheme.js.✅ What I liked~
LiveValueisinternal— it's a mechanism of this library's inputs, not a public API. Good architectural boundary.LiveValueTests+LiveValueInteropTestsare genuine state-machine tests, not tautologies — they drive the decision through odd sequences (echo-that-lags, value-the-element-already-holds, unrendered-element-keeps-push-for-later, torn-down-circuit-swallows-push) and assert directional outcomes.A_keystroke_is_never_echoed_back_into_the_render_tree— yes, that's exactly what it pins.clearComposercomment fix and the README update show care for the docs surface, not just the code.Automated review by Jibril · 2026-07-28
CI/CD: passed for head
b856ce7(forgejo-actions coverage #4787, 94.8%/88.2%) · Local checks: 275/275 pass, build 0 warnings/0 errors