From 4eb4196bcddebefebbfb77b6d3bcb10e020537ed Mon Sep 17 00:00:00 2001 From: Johan Lindh Date: Fri, 7 Aug 2026 12:20:10 +0200 Subject: [PATCH] fix(bind): keep the numeric setter adapter transparent 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. --- lib/bind/setterfloat64.go | 56 +++++++++- lib/bind/setterfloat64_test.go | 183 ++++++++++++++++++++++++++++++++- lib/ui/input_widgets_test.go | 69 +++++++++++++ 3 files changed, 305 insertions(+), 3 deletions(-) diff --git a/lib/bind/setterfloat64.go b/lib/bind/setterfloat64.go index a66b37e9..97c11517 100644 --- a/lib/bind/setterfloat64.go +++ b/lib/bind/setterfloat64.go @@ -3,6 +3,7 @@ package bind import ( "errors" "fmt" + "html/template" "math" "strconv" @@ -22,10 +23,50 @@ type numeric interface { ~uint | ~uint8 | ~uint16 | ~uint32 | ~uint64 | ~uintptr } +// setterFloat64 adapts a numeric [Setter] to a Setter[float64], sanitizing every +// write through sanitizeFloatForT. +// +// Embedding Setter[T] promotes only JawsGet and JawsSet, so the adapter forwards +// the optional event and attribute interfaces explicitly. Without that, wrapping +// would hide them from [jaws.Element.ApplyGetter], silently dropping the Clicked, +// ContextMenu and InitialHTMLAttr hooks a [Binder] exposes. Each forward reports +// "not handled" when the wrapped Setter does not implement the interface, which is +// what ApplyGetter and the event dispatch already expect for a plain setter. type setterFloat64[T numeric] struct { Setter[T] } +func (s setterFloat64[T]) JawsClick(elem *jaws.Element, click jaws.Click) (err error) { + err = jaws.ErrEventUnhandled + if h, ok := s.Setter.(jaws.ClickHandler); ok { + err = h.JawsClick(elem, click) + } + return +} + +func (s setterFloat64[T]) JawsContextMenu(elem *jaws.Element, click jaws.Click) (err error) { + err = jaws.ErrEventUnhandled + if h, ok := s.Setter.(jaws.ContextMenuHandler); ok { + err = h.JawsContextMenu(elem, click) + } + return +} + +func (s setterFloat64[T]) JawsInput(elem *jaws.Element, value string) (err error) { + err = jaws.ErrEventUnhandled + if h, ok := s.Setter.(jaws.InputHandler); ok { + err = h.JawsInput(elem, value) + } + return +} + +func (s setterFloat64[T]) JawsInitialHTMLAttr(elem *jaws.Element) (attr template.HTMLAttr) { + if h, ok := s.Setter.(jaws.InitialHTMLAttrHandler); ok { + attr = h.JawsInitialHTMLAttr(elem) + } + return +} + // sanitizeFloatForT validates value before it is converted to T and reports // whether it may be the canonical float64 view of more than one T integer. It // rejects non-finite values for every numeric T, and for integer T also rejects @@ -193,6 +234,13 @@ func makeSetterFloat64for[T numeric](s *Setter[float64], value any) bool { // bridge can lose precision: not every integer magnitude beyond 2^53 is exactly // representable as float64. // +// Writes are sanitized whatever the bound type. [Setter.JawsSet] returns +// [ErrFloatNotFinite] for a NaN or infinite value and [ErrFloatOutOfRange] for a +// finite value that does not fit the bound type, leaving the bound value +// unchanged in both cases. The float64 case is sanitized too, so an untrusted +// value cannot store a NaN that would defeat the equality comparison every +// change-detecting setter relies on. +// // When an integer loses precision in that conversion, writing the canonical // float64 returned by [Getter.JawsGet] back through [Setter.JawsSet] preserves // the underlying integer and returns [jaws.ErrValueUnchanged]. This also applies @@ -216,7 +264,13 @@ func makeSetterFloat64for[T numeric](s *Setter[float64], value any) bool { func MakeSetterFloat64(value any) (s Setter[float64]) { switch v := value.(type) { case Setter[float64]: - return v + // Wrap rather than pass through: setterFloat64 is where the non-finite guard + // lives, and a float64 setter is the one settable case that needs no + // conversion, so passing it through unchanged would be the only binding that + // accepts NaN. The wrapper costs nothing else here: sanitizeFloatForT[float64] + // takes the finiteness-only default branch and reports mayAlias false, and + // T(value) is the identity, so no conversion or extra JawsGet is introduced. + return setterFloat64[float64]{Setter: v} case Getter[float64]: return setterReadOnly[float64]{v} case float64: diff --git a/lib/bind/setterfloat64_test.go b/lib/bind/setterfloat64_test.go index d6ace41b..5f365e94 100644 --- a/lib/bind/setterfloat64_test.go +++ b/lib/bind/setterfloat64_test.go @@ -2,6 +2,7 @@ package bind import ( "errors" + "html/template" "math" "reflect" "strconv" @@ -22,6 +23,7 @@ func (tg testGetter[T]) JawsGet(elem *jaws.Element) T { } func Test_makeSetterFloat64types(t *testing.T) { + tsfloat64 := newTestSetter(float64(0)) tsint := newTestSetter(int(0)) tsuintptr := newTestSetter(uintptr(0)) tests := []struct { @@ -31,8 +33,8 @@ func Test_makeSetterFloat64types(t *testing.T) { }{ { name: "Setter[float64]", - v: setterFloat64[float64]{}, - wantS: setterFloat64[float64]{}, + v: tsfloat64, + wantS: setterFloat64[float64]{tsfloat64}, }, { name: "Getter[float64]", @@ -569,3 +571,180 @@ func Test_makeSetterFloat64_panicNamedNumeric(t *testing.T) { assertPanics("value", Celsius(20)) assertPanics("setter", newTestSetter[Celsius](20)) } + +// Test_setterFloat64_rejectsNonFiniteForEveryNumericType pins that +// [MakeSetterFloat64] applies the non-finite guard uniformly, whatever the bound +// numeric type. float64 is the case worth pinning: it is the only settable type +// that reaches JawsSet without a conversion step, so a missing guard there would +// store a NaN that permanently defeats the equality-based update dedup in +// binder.JawsSetLocked (NaN != NaN) and, for the float widgets in lib/ui, would +// terminate every Request that later renders it. +func Test_setterFloat64_rejectsNonFiniteForEveryNumericType(t *testing.T) { + nonFinite := []float64{math.NaN(), math.Inf(1), math.Inf(-1)} + + assertRejects := func(t *testing.T, name string, s Setter[float64], unchanged func() bool) { + t.Helper() + for _, bad := range nonFinite { + if err := s.JawsSet(nil, bad); !errors.Is(err, ErrFloatNotFinite) { + t.Errorf("%s: JawsSet(%v) = %v, want ErrFloatNotFinite", name, bad, err) + } + if !unchanged() { + t.Fatalf("%s: JawsSet(%v) mutated the bound value", name, bad) + } + } + } + + t.Run("float64", func(t *testing.T) { + var mu sync.Mutex + value := 1.5 + assertRejects(t, "float64", MakeSetterFloat64(New(&mu, &value)), func() bool { + return value == 1.5 + }) + }) + + t.Run("float32", func(t *testing.T) { + var mu sync.Mutex + value := float32(1.5) + assertRejects(t, "float32", MakeSetterFloat64(New(&mu, &value)), func() bool { + return value == 1.5 + }) + }) + + t.Run("int", func(t *testing.T) { + var mu sync.Mutex + value := 3 + assertRejects(t, "int", MakeSetterFloat64(New(&mu, &value)), func() bool { + return value == 3 + }) + }) +} + +// Test_setterFloat64_float64PassThroughBehavior pins the behavior the non-finite +// guard must not disturb for a plain float64 binding: finite values are stored, +// an unchanged value still reports jaws.ErrValueUnchanged, and the setter still +// resolves to the same tag key as the underlying Binder. +func Test_setterFloat64_float64PassThroughBehavior(t *testing.T) { + var mu sync.Mutex + value := 0.0 + bind := New(&mu, &value) + s := MakeSetterFloat64(bind) + + if err := s.JawsSet(nil, 2.5); err != nil { + t.Fatalf("JawsSet(2.5): %v", err) + } + if value != 2.5 { + t.Fatalf("stored value = %v, want 2.5", value) + } + if got := s.JawsGet(nil); got != 2.5 { + t.Fatalf("JawsGet() = %v, want 2.5", got) + } + if err := s.JawsSet(nil, 2.5); !errors.Is(err, jaws.ErrValueUnchanged) { + t.Fatalf("JawsSet(2.5) again = %v, want ErrValueUnchanged", err) + } + + tags, err := tag.TagExpand(s) + if err != nil { + t.Fatalf("TagExpand: %v", err) + } + if len(tags) != 1 || tags[0] != any(&value) { + t.Fatalf("TagExpand() = %#v, want [%p]", tags, &value) + } +} + +// inputSetterFloat64 is a Setter[float64] that also handles raw browser input, so +// the tests can exercise setterFloat64's InputHandler forwarding. bind.Binder +// covers the click, context-menu and initial-attribute interfaces but not this one. +type inputSetterFloat64 struct { + *testSetter[float64] + got string +} + +func (is *inputSetterFloat64) JawsInput(elem *jaws.Element, value string) error { + is.got = value + return nil +} + +// Test_setterFloat64_forwardsOptionalInterfaces pins that adapting a numeric +// Setter stays transparent to jaws.Element.ApplyGetter. The adapter embeds +// Setter[T], which promotes only JawsGet and JawsSet, so the optional event and +// attribute interfaces must be forwarded explicitly or a Binder's Clicked, +// ContextMenu and InitialHTMLAttr hooks would silently stop firing once the value +// is bound to a float widget. +func Test_setterFloat64_forwardsOptionalInterfaces(t *testing.T) { + t.Run("delegates to the wrapped setter", func(t *testing.T) { + var mu sync.Mutex + value := 1.5 + var clicked, contextMenu int + b := New(&mu, &value). + Clicked(func(Binder[float64], *jaws.Element, jaws.Click) error { + clicked++ + return nil + }). + ContextMenu(func(Binder[float64], *jaws.Element, jaws.Click) error { + contextMenu++ + return nil + }). + InitialHTMLAttr(func(Binder[float64], *jaws.Element) template.HTMLAttr { + return `data-x="1"` + }) + s := MakeSetterFloat64(b) + + ch, ok := s.(jaws.ClickHandler) + if !ok { + t.Fatal("adapter does not implement jaws.ClickHandler") + } + if err := ch.JawsClick(nil, jaws.Click{}); err != nil || clicked != 1 { + t.Errorf("JawsClick() = %v, clicked = %d, want nil and 1", err, clicked) + } + + cm, ok := s.(jaws.ContextMenuHandler) + if !ok { + t.Fatal("adapter does not implement jaws.ContextMenuHandler") + } + if err := cm.JawsContextMenu(nil, jaws.Click{}); err != nil || contextMenu != 1 { + t.Errorf("JawsContextMenu() = %v, contextMenu = %d, want nil and 1", err, contextMenu) + } + + ah, ok := s.(jaws.InitialHTMLAttrHandler) + if !ok { + t.Fatal("adapter does not implement jaws.InitialHTMLAttrHandler") + } + if got := ah.JawsInitialHTMLAttr(nil); got != `data-x="1"` { + t.Errorf("JawsInitialHTMLAttr() = %q, want %q", got, `data-x="1"`) + } + }) + + t.Run("forwards input to a wrapped InputHandler", func(t *testing.T) { + is := &inputSetterFloat64{testSetter: newTestSetter(1.5)} + s := MakeSetterFloat64(is) + ih, ok := s.(jaws.InputHandler) + if !ok { + t.Fatal("adapter does not implement jaws.InputHandler") + } + if err := ih.JawsInput(nil, "2.5"); err != nil { + t.Fatalf("JawsInput(): %v", err) + } + if is.got != "2.5" { + t.Errorf("wrapped JawsInput got %q, want %q", is.got, "2.5") + } + }) + + t.Run("reports unhandled for a plain setter", func(t *testing.T) { + // A Setter that implements none of the optional interfaces must leave the + // event unhandled so dispatch falls through to the widget, and must + // contribute no initial attribute. + s := MakeSetterFloat64(newTestSetter(1.5)) + if err := s.(jaws.ClickHandler).JawsClick(nil, jaws.Click{}); !errors.Is(err, jaws.ErrEventUnhandled) { + t.Errorf("JawsClick() = %v, want ErrEventUnhandled", err) + } + if err := s.(jaws.ContextMenuHandler).JawsContextMenu(nil, jaws.Click{}); !errors.Is(err, jaws.ErrEventUnhandled) { + t.Errorf("JawsContextMenu() = %v, want ErrEventUnhandled", err) + } + if err := s.(jaws.InputHandler).JawsInput(nil, "1"); !errors.Is(err, jaws.ErrEventUnhandled) { + t.Errorf("JawsInput() = %v, want ErrEventUnhandled", err) + } + if got := s.(jaws.InitialHTMLAttrHandler).JawsInitialHTMLAttr(nil); got != "" { + t.Errorf("JawsInitialHTMLAttr() = %q, want empty", got) + } + }) +} diff --git a/lib/ui/input_widgets_test.go b/lib/ui/input_widgets_test.go index 5f6208f2..57990e08 100644 --- a/lib/ui/input_widgets_test.go +++ b/lib/ui/input_widgets_test.go @@ -631,3 +631,72 @@ func TestInputTextWidget_InitialHTMLAttrFromBinder(t *testing.T) { _, got := renderUI(t, rq, NewText(b)) mustMatch(t, `^$`, got) } + +// TestInputFloat_BinderAdapterStaysTransparent guards every binding that reaches a +// float widget through bind.MakeSetterFloat64, which wraps the Binder in an adapter +// so non-finite browser values are rejected. The adapter must not shadow what +// jaws.Element.ApplyGetter looks for: the Binder's Clicked and InitialHTMLAttr hooks +// and its pointer-derived dirty tag all have to survive the wrapping. An integer +// binding is covered as well as float64, because it takes the converting path. +func TestInputFloat_BinderAdapterStaysTransparent(t *testing.T) { + t.Run("float64", func(t *testing.T) { + _, rq := newCoreRequest(t) + var mu deadlock.Mutex + value := 1.5 + var clicked int + b := bind.New(&mu, &value). + Clicked(func(bind.Binder[float64], *jaws.Element, jaws.Click) error { + clicked++ + return nil + }). + InitialHTMLAttr(func(bind.Binder[float64], *jaws.Element) template.HTMLAttr { + return `data-unit="cm"` + }) + number := NewNumber(bind.MakeSetterFloat64(b)) + + elem, got := renderUI(t, rq, number) + mustMatch(t, `^$`, got) + + // The dirty target must still be the bound pointer, so a browser edit + // reconciles through the same tag the application dirties. + if !elem.HasTag(&value) { + t.Fatalf("element not registered under the bound pointer; tags: %v", rq.TagsOf(elem)) + } + if err := jaws.CallEventHandlers(elem.UI(), elem, what.Click, "0 0 0 x"); err != nil { + t.Fatalf("click: %v", err) + } + if clicked != 1 { + t.Fatalf("Clicked hook fired %d times, want 1", clicked) + } + + if err := number.JawsInput(elem, "2.5"); err != nil { + t.Fatalf("JawsInput(2.5): %v", err) + } + if value != 2.5 { + t.Fatalf("stored value = %v, want 2.5", value) + } + + // A non-finite value is refused, leaving the bound value intact rather than + // storing a NaN that would break change detection. + if err := bind.MakeSetterFloat64(b).JawsSet(elem, math.NaN()); !errors.Is(err, bind.ErrFloatNotFinite) { + t.Fatalf("JawsSet(NaN) = %v, want bind.ErrFloatNotFinite", err) + } + if value != 2.5 { + t.Fatalf("stored value = %v after NaN, want 2.5", value) + } + }) + + t.Run("int", func(t *testing.T) { + _, rq := newCoreRequest(t) + var mu deadlock.Mutex + value := 3 + b := bind.New(&mu, &value).InitialHTMLAttr( + func(bind.Binder[int], *jaws.Element) template.HTMLAttr { return `data-unit="px"` }) + + elem, got := renderUI(t, rq, NewNumber(bind.MakeSetterFloat64(b))) + mustMatch(t, `^$`, got) + if !elem.HasTag(&value) { + t.Fatalf("element not registered under the bound pointer; tags: %v", rq.TagsOf(elem)) + } + }) +}