Repository navigation
Add native validation transaction - #167
Merged
Merged
Conversation
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
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
Add native Control validation through one
Control.ValidateCore, shared by #160's canonical focus preflight,Control.Validate(),Control.ValidateChildren()andWindowBase.ValidateChildren(). Public API includesValidating,Validated,CausesValidationand protected event raisers, with XML docs, examples and a complete audit.Root cause and ordering
Binding.DataSourceUpdateMode.OnValidationwas the default, but Control had no validation lifecycle. Editing a bound TextBox and leaving it therefore did not update the source.The accepted departure now follows:
Validating → OnValidation binding writes → Validated → canonical focus commit → existing text-input handoff → LostFocus → current GotFocusThe old control remains canonical owner with its text-input session during validation. User cancellation prevents validation binding writes and the focus transition. Failure or a stale request suppresses completion/commit; already executed callback or setter side effects are not rolled back. Destination
CausesValidation=falseskips automatic departure validation, while explicit validation remains available.Binding and failure policy
Explicit API and traversal
Validate uses the same routine without moving focus or transferring text input. Live detached/hidden/disabled controls can validate; disposed/retiring controls and same-control recursion return false. UI-thread access is enforced.
ValidateChildren snapshots public descendants in depth-first preorder and public collection order, excluding the receiver and implicit framework subtrees. Form includes client children, not its implicit title bar. Hidden/disabled fields are included; disposed/moved entries are skipped, additions deferred, first failure stops, and exceptions propagate. The inheritance tree is unchanged.
Reentrancy, input and Designer
Nested validating destinations are coalesced to the latest valid request. Generation/ancestry checks prevent stale continuations; selecting the owner cancels departure. A bypass request can commit immediately, and mandatory retirement always bypasses validation. Owner writes and focus notifications stay in #160's existing canonical transaction.
Tab, pointer, accessibility keyboard focus and automation use the same preflight. Canceled/throwing pointer validation cannot leave a release click. Cancellation retains the editor session/composition; accepted commit uses the existing handoff. Android screen-reader focus remains separate, with no new backend validation or IME path.
Designer discovers inherited Validating/Validated through runtime metadata, verified by an Events-view regression. No hard-coded event list or Designer validation engine is added.
Validation
Final review corrected two reproduced local blockers (shared-source traversal overwriting a later edit, and a pending mode change suppressing OnPropertyChanged). Eight additional regression cases cover both roots and cleanup. Fresh validation after those changes:
Full-suite breakdown: Automation 166, Automation.Windows 66, CrossPlatform sample 29, Designer 691, Testing 927, Framework 1482, VSIX 26, Android backend 358, Windows backend 328; all passed.
Build/test execution used sequential MSBuild (
-m:1 /p:UseSharedCompilation=false). NativeUiAutomationHost --validationchecks accepted writes/handoff and cancellation/session retention. All 18 changed C# files match the final local validation source snapshot.Android emulator, physical Android device, manual CJK IME, interactive Visual Studio and manual ControlGallery: NOT EXECUTED. Backend tests are not device qualification, and the HWND smoke is not manual installed-IME acceptance. Template verification was unnecessary because startup/templates did not change.
Retained intermittent allocation result
An earlier full implementation run failed only
BrushInterpolationCompatibilityTests.PreparedPlanDoesNotAllocatePerIntermediateFrameat 6280 B / 256 B. The same test had failed before #161 during #160 (496 B / 256 B). Its test and animation/brush sources are unchanged. Five independent isolated executions then passed, followed by a complete 4065/4065 run. The fresh final-review suite also passed it within 4073/4073. No threshold, test, retry policy or animation code was changed. The exact allocation source remains a separate diagnostic follow-up.Current GitHub Actions result
Required
build: SUCCESS in run 37206449059, attempt 2 / job 111458432849, on unchanged head8d7d88372491f9a017b6b5972159fb9aa3214b3e. Restore, Release build and the complete Release suite passed: 4073/4073, 0 skipped. Exactly one normal rerun was made after the diagnostic evidence below; no source, test, threshold, runtime or workflow setting was changed.Attempt 1 is retained: 4072/4073 PASS, sole failure
PerformanceProfilerTests.RecordingOnlyHotPathDoesNotAllocateAfterSourceWarmup, 3256 B vs 0 B. This is separate from the brush-allocation failure above. The exact allocation source remains unknown; the successful rerun does not establish its cause.Diagnosis found a confirmed failure of the same test before #161: run 35512235500 attempt 1, 2026-09-20, head
8ccbc8f5d277d09899d2e45cce7c01b20725a26e, 1328 B vs 0 B. That commit is an ancestor of exact base984fbca99a77f165bae36459e2eae131dcf68c04; profiler/shared diagnostics/test sources have no diff from it to base or from base to this PR.Current local comparisons on exact base and PR each passed 20 fresh isolated processes, 10 full-class processes, 3 full-project processes, plus 2 full-project processes at 4 CPUs. Both versions also passed 5 fresh processes per runtime variant (4 CPUs: defaults, tiered compilation off, PGO off, ReadyToRun off, Server GC). PR full Release solution passed 4073/4073. The original zero-byte assertion was unchanged. Current exact-base local failure was not reproduced. Thread instrumentation in an ignored copy confirmed one thread through warmup/measurement and no active profiler before/after; no deterministic predecessor or state leak was found. Exact CI predecessor/allocation stack is unavailable.
The measured loop does not invoke validation/focus/binding. New tests can affect scheduling/initialization timing, so an indirect timing effect is not categorically excluded. No deterministic #161-specific regression was identified. Historical evidence supports a pre-existing failure pattern; JIT/first-use/GC attribution remains unproven. Follow-up investigation and any justified separate fix are tracked in #168. All diagnostic helpers/logs/probes are ignored local artifacts, outside this PR. The PR remains open for final merge review; no merge was performed.
Scope and issue links
22 implementation/test/documentation files only. Local artifacts, logs, TRX, PR helpers, generated outputs and
.codex/config.tomlare excluded. Full contract and evidence: validation guide and audit.No ContainerControl, AutoValidate, ValidationConstraints, rollback subsystem, focus/IME redesign or unrelated issue implementation. This PR is intentionally left open for final review; merge is outside this task.
Fixes #161
Related to #144
Related to #146