fix(bind): keep the numeric setter adapter transparent - #245
Closed
linkdata wants to merge 1 commit into
Closed
Conversation
setterFloat64 embeds Setter[T], which promotes only JawsGet and JawsSet, so Element.ApplyGetter never saw a wrapped Binder's JawsClick, JawsContextMenu or JawsInitialHTMLAttr. Every integer, float32 and uintptr binding silently dropped those hooks: rw.Number(bind.New(&mu, &n).InitialHTMLAttr(fn)) // fn never called Forward the four optional interfaces explicitly, each reporting the event as unhandled when the wrapped Setter does not implement it, which is what ApplyGetter and the event dispatch already expect for a plain setter. MakeSetterFloat64 also returned a Setter[float64] unwrapped, making it the one settable type that never reached sanitizeFloatForT: float32 and the integer types rejected NaN and Inf while float64 stored them. A stored NaN permanently defeats the equality comparison binder.JawsSetLocked uses to detect change, and makes ui.Number and ui.Range cancel the Request on the next render. Wrap it like every other numeric type, which costs no behavior now that the adapter is transparent.
Owner
Author
|
Closing as invalid in favor of #254. Wrapping an existing Setter[float64] changes its dynamic interface set, breaks Binder HTMLGetter/Format composition, and makes plain setters appear to be event handlers. InputFloat already enforces finite values at the UI boundary. The remaining converted-Binder behavior issue is tracked in #254. |
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.
Two defects in
bind.MakeSetterFloat64's adapter, with one root cause: the adapter is not transparent tojaws.Element.ApplyGetter.1. Binder hooks silently dropped (the user-visible one)
setterFloat64[T]embedsSetter[T], which promotes onlyJawsGetandJawsSet.ApplyGettertherefore never sees the wrappedBinder'sJawsClick,JawsContextMenuorJawsInitialHTMLAttr, and every integer,float32anduintptrbinding loses them without a diagnostic:Fixed by forwarding the four optional interfaces explicitly. Each reports the event as unhandled when the wrapped
Setterdoes not implement it, which is whatApplyGetterand the event dispatch already expect for a plain setter, so dispatch still falls through to the widget.2.
float64writes were never sanitizedMakeSetterFloat64returned aSetter[float64]unwrapped, so it was the one settable type that never reachedsanitizeFloatForT.float32and the integer types rejectNaN/Inf;float64stored them:A stored
NaNpermanently defeats thevalue != *b.ptrcomparisonbinder.JawsSetLockeduses to detect change, so every later set reports "changed" and re-dirties — precisely the failure the guard exists to prevent. For a value bound toui.Number/ui.Rangeit then cancels the Request on the next render or update.float64escaped the guard for the same reason it escaped #1: it was passed through instead of wrapped. Wrapping it like every other numeric type costs no behavior now that the adapter is transparent —sanitizeFloatForT[float64]takes the finiteness-only default branch, reportsmayAliasfalse, andT(value)is the identity, so no conversion or extraJawsGetis introduced.MakeSetterFloat64's doc now states the rejection contract, since callers can seeErrFloatNotFinite/ErrFloatOutOfRangefrom any binding.lib/uiwidgets were not affected by #2 in practice:InputFloatre-checks finiteness on all three entry points before calling the setter. Anyone building directly onbindhad no such guard.Scope note
setterFloat64ReadOnly[T]has the same opaque-wrapper shape forGettervalues. It is left alone here: aBinderalways matches theSettercase first, so nothing regresses through it, but it is the same latent gap.Tests
lib/bind: non-finite parity acrossfloat64/float32/int; float64 pass-through behavior includingErrValueUnchangedand tag identity throughTagExpand; interface forwarding in all three states (delegating, input-handler, plain setter).lib/ui: end-to-end guard that aBinder'sClicked/InitialHTMLAttrand its pointer-derived dirty tag survive the adapter, for both thefloat64and the convertingintpath.Each new test was confirmed to fail on
mainfor the stated reason before the fix.Verification
go generate,go vet,gofmt -l .,staticcheck,golangci-lint run(0 issues),go test -race ./...,go test -tags debug -race ./..., and the production (non-race) leg all pass. Statement coverage stays at 100% in bothlib/bindandlib/ui. The 386 leg compiles and vets locally; it cannot execute in this environment, so CI covers it.