Conversation
…into fix-dynamic-input-type
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Source-only review of frozen head 6cea93e0a2e30e076a6f68d3cc31df18e9f78242 found two P2 regressions: empty string checkbox values are conflated with Boolean true, and deferred input values survive their render batch and can overwrite newer client edits when type changes.
Both routed guides (docs/CrossCuttingGuidance.md and docs/BlazorComponentsGuidance.md) completed independently against the version-2 frozen bundle. No prior feedback duplicated these findings, no candidates remain unresolved, and no materially changed areas were left uncovered. Security assessment and execution of PR code, tests, CI, browser workflows, and samples were explicitly excluded.
Provenance: source-only proof from immutable guidance commit d56fcef3113bc46e60cb7a32ea8227ce9133e86f, merge base/base tip bf676af7baa13fa4d06be384b4285d5406161219, and frozen head above. No runtime test claims are made. Evidence is limited to tracked frozen bytes; symlink, submodule, and LFS targets were not used, and worker runtime model metadata was unavailable.
Generated by ASP.NET Core Pull Request Review for #119 · copilot · gpt56 · 324.9 AIC · ⌖ 11.5 AIC · ⊞ 18.7K
Comment /review to run again
| } | ||
|
|
||
| // Blazor wire-format encodes bool as '' (true) or null (false); any other string is a form-post value and must go to .value. | ||
| if (element instanceof HTMLInputElement && element.type === 'checkbox' && (value === '' || value === null)) { |
There was a problem hiding this comment.
Empty string checkbox values are now interpreted as checked state instead of as the element's form-post value.
src/Components/Web.JS/src/Rendering/DomSpecialPropertyUtil.ts:191 — P2
Minimal consumer repro: render <input type="checkbox" value=""> (or call RenderTreeBuilder.AddAttribute(..., "value", "")). The string overload and Boolean true both reach this line as "", so the checkbox becomes checked and its .value remains the default/stale value; previously .value became empty and checked state was unchanged.
The fix needs to preserve the attribute value's kind through the render-batch wire format instead of inferring Boolean state from ""/null.
| } | ||
|
|
||
| if (deferredValue !== undefined) { | ||
| setDeferredElementValue(inputElement, deferredValue); |
There was a problem hiding this comment.
Replaying the never-cleared deferred value on later type edits overwrites newer client input.
src/Components/Web.JS/src/Rendering/DomSpecialPropertyUtil.ts:44 — P2
Minimal consumer repro: bind <input type="@type" @bind="text">, edit and blur it, then toggle type between text and password. Event dispatch patches the render tree so no value edit is emitted, but _blazorDeferredValue still contains the prior rendered text; this call restores that stale text when only type changes.
Scope the deferred value to the render batch (and clear it after insertion/update processing) before replaying it here.
This branch has not been deployed
2 participants
Fork-internal hosted /review validation of dotnet/aspnetcore dotnet#69634 at 3d47844. Exact upstream dotnet#67716 head 6cea93e against merge-base bf676af. All validation writes stay inside PureWeen/aspnetcore.