...
func (rw RequestWriter) Register(updater jaws.Updater, params ...any) jid.Jid {
+ // Create and report the Element rather than going through RequestWriter.NewUI, so
+ // a surrounding Template owns it without anything being rendered: NewUI calls
+ // JawsRender, which appends a debug comment when Jaws.Debug is set, and the
+ // documented usage puts the returned Jid inside an attribute
+ // (), where that comment would corrupt the markup.
elem := rw.NewElement(Register{Updater: updater})
+ rw.trackElement(elem)
elem.Tag(updater)
// The wrapping Register element's UI is not the updater, so events reach the
// updater only through the element's handler list, not the elem.UI() fallback.
diff --git a/lib/ui/requestwriter.go b/lib/ui/requestwriter.go
index e27d491c..a15bee1a 100644
--- a/lib/ui/requestwriter.go
+++ b/lib/ui/requestwriter.go
@@ -11,15 +11,23 @@ import (
type RequestWriter struct {
*jaws.Request
io.Writer
- // elementRendered, when non-nil, is called by NewUI with each Element it
- // successfully rendered, letting the widget that owns this writer track the
- // Elements created through it. A returned error fails the NewUI call.
+ // elementCreated, when non-nil, is called with every Element created through
+ // this writer, letting the widget that owns the writer track them. It is called
+ // as soon as the Element exists, before it renders and whether or not it ever
+ // does, so an implementation must not assume rendered state.
//
// It is deliberately unexported: RequestWriter is embedded in [With], so an
// exported func field would be callable from any template as
- // {{call $.ElementRendered $.Element}}, letting a template make an Element own
+ // {{call $.ElementCreated $.Element}}, letting a template make an Element own
// itself and have its own wrapper unregistered on the next update.
- elementRendered func(elem *jaws.Element) (err error)
+ elementCreated func(elem *jaws.Element)
+}
+
+// trackElement reports a newly created Element to the writer's owner, if any.
+func (rw RequestWriter) trackElement(elem *jaws.Element) {
+ if rw.elementCreated != nil {
+ rw.elementCreated(elem)
+ }
}
// NewUI creates an element for ui and renders it to the underlying writer.
@@ -28,12 +36,12 @@ type RequestWriter struct {
// requirements documented by [jaws.UI].
func (rw RequestWriter) NewUI(ui jaws.UI, params ...any) (err error) {
elem := rw.NewElement(ui)
- if err = elem.JawsRender(rw, params); err == nil {
- if rw.elementRendered != nil {
- err = rw.elementRendered(elem)
- }
- }
- if err != nil {
+ // Report the Element before rendering it, so the owner's set is complete even for
+ // one that fails. That set may then hold an Element already unregistered below,
+ // which costs nothing: Request.DeleteElements skips elements it finds
+ // unregistered, and every rollback path deletes the whole set at once.
+ rw.trackElement(elem)
+ if err = elem.JawsRender(rw, params); err != nil {
// Unregister anything the failed Element already owns along with it, so no
// widget can strand a subtree by not rolling back itself.
deleteOwnedElements(rw.Request, []*jaws.Element{elem})
diff --git a/lib/ui/template.go b/lib/ui/template.go
index b57eb388..8a1d08fb 100644
--- a/lib/ui/template.go
+++ b/lib/ui/template.go
@@ -28,20 +28,19 @@ import (
// helper. The referenced template must be a partial template, not a full HTML
// document.
//
-// The Elements a template creates through [RequestWriter.NewUI] — the path taken by
-// every RequestWriter widget helper except [RequestWriter.Register] and
-// [RequestWriter.RadioGroup], including a nested [RequestWriter.Template] — belong to
-// the Template that rendered them: when [Template.JawsUpdate] replaces the wrapper's
-// content, those Elements are unregistered along with the DOM that held them, and a
-// nested widget's own Elements go with it.
+// Every Element a template creates through the [RequestWriter] it is given belongs to
+// the Template that rendered it — the widget helpers, [RequestWriter.Register],
+// [RequestWriter.RadioGroup] and a nested [RequestWriter.Template] alike. When
+// [Template.JawsUpdate] replaces the wrapper's content, those Elements are
+// unregistered along with the DOM that held them, and a nested widget's own Elements
+// go with it.
//
-// Those two helpers create their Elements through [jaws.Request.NewElement] instead,
-// so a Template neither tracks nor unregisters them. Their cleanup is left to the
-// browser, which reports the JaWS ids it removed from the DOM as the wrapper's new
-// content is applied, and the [jaws.Request] unregisters those Elements. An Element
-// whose id never reaches the DOM has nothing to report it and stays registered until
-// the Request ends: one from an execution that failed before its markup was
-// delivered, or from a Register call whose returned Jid the template discards.
+// Ownership is recorded when an Element is created rather than after it renders, so
+// one that never reaches the browser is reclaimed too: an Element whose render failed,
+// or a radio Element left unrendered by a [RadioElement.Label] without its
+// [RadioElement.Radio]. See [RequestWriter.RadioGroup] for the one attribution
+// condition, which applies when a group's markup ends up in a different wrapper than
+// the RadioGroup call.
//
// Template execution is best-effort rather than transactional. Template actions
// and nested JaWS helpers run as the template executes, so an execution error
@@ -81,12 +80,12 @@ func (tmpl *Template) String() string {
}
// ownElement records child as created while tmpl's template executed. It is the
-// [RequestWriter] element-rendered hook installed by execute.
-func (tmpl *Template) ownElement(child *jaws.Element) (err error) {
+// [RequestWriter] element-created hook installed by execute, so it is called as soon
+// as the Element exists and makes no assumption about whether it rendered.
+func (tmpl *Template) ownElement(child *jaws.Element) {
tmpl.mu.Lock()
tmpl.owned = append(tmpl.owned, child)
tmpl.mu.Unlock()
- return
}
// takeOwnedElements returns the Elements created by the most recent execution and
@@ -137,7 +136,7 @@ func (tmpl *Template) execute(elem *jaws.Element, w io.Writer, lookedUp *templat
// races on the shared io.Writer, and on the render as a whole.
err = lookedUp.Execute(w, With{
Element: elem,
- RequestWriter: RequestWriter{Request: elem.Request, Writer: w, elementRendered: tmpl.ownElement},
+ RequestWriter: RequestWriter{Request: elem.Request, Writer: w, elementCreated: tmpl.ownElement},
Dot: tmpl.Dot,
Auth: tmpl.auth(elem),
})
diff --git a/lib/ui/template_owned_test.go b/lib/ui/template_owned_test.go
index 583796a3..3ac81365 100644
--- a/lib/ui/template_owned_test.go
+++ b/lib/ui/template_owned_test.go
@@ -4,6 +4,7 @@ import (
"errors"
"fmt"
"html/template"
+ "slices"
"strings"
"sync"
"testing"
@@ -28,6 +29,12 @@ const ownedTestTemplates = `
{{define "owned-container"}}{{$.RequestWriter.Container "div" $.Dot.Container}}{{end}}
{{define "owned-register"}}
{{end}}
{{define "owned-radiogroup"}}{{range $.RequestWriter.RadioGroup $.Dot.Radios}}{{.Radio}}{{.Label}}{{end}}{{end}}
+{{define "owned-register-discarded"}}registered as text: {{$.RequestWriter.Register $.Dot}}{{end}}
+{{define "owned-labelonly"}}{{range $.RequestWriter.RadioGroup $.Dot.Radios}}{{.Label}}{{end}}{{end}}
+{{define "owned-register-failafter"}}
{{$.Dot.Check}}{{end}}
+{{define "owned-radiogroup-failafter"}}{{range $.RequestWriter.RadioGroup $.Dot.Radios}}{{.Radio}}{{.Label}}{{end}}{{$.Dot.Check}}{{end}}
+{{define "owned-radio-outer"}}{{$.RequestWriter.Template "div" "owned-radio-inner" ($.Dot.Box $.RequestWriter)}}{{end}}
+{{define "owned-radio-inner"}}{{range $.Dot.Elements}}{{.Radio}}{{.Label}}{{end}}{{end}}
`
var errOwnedDotCheck = errors.New("owned dot check failed")
@@ -40,6 +47,7 @@ type ownedDot struct {
container *testContainer
names []string
radios *named.BoolArray
+ box *ownedRadioBox
}
func (d *ownedDot) Check() (string, error) {
@@ -60,6 +68,33 @@ func (d *ownedDot) Names() []string { return d.names }
func (d *ownedDot) Radios() *named.BoolArray { return d.radios }
+// Box builds the radio group with the passed-in writer — the outer template's — and
+// returns the box carrying the RadioElement values to the nested template, whose dot it
+// becomes. It is a pointer so it is usable as a tag.
+func (d *ownedDot) Box(rw RequestWriter) *ownedRadioBox {
+ d.box.rel = rw.RadioGroup(d.radios)
+ return d.box
+}
+
+// ownedRadioBox carries RadioElement values across a template boundary, so a test can
+// render them in a nested template while the outer template owns their Elements.
+type ownedRadioBox struct {
+ rel []RadioElement
+ show bool
+ execs int // executions of the nested template that read Elements
+}
+
+// Elements returns the group to render, or nothing once show is cleared. It counts
+// calls so a test can tell an update that ran and rendered nothing from one that never
+// executed.
+func (b *ownedRadioBox) Elements() []RadioElement {
+ b.execs++
+ if !b.show {
+ return nil
+ }
+ return b.rel
+}
+
// JawsUpdate makes ownedDot usable as the $.Register updater. Register tags its
// Element with the updater, so the dot still tags the whole subtree.
func (d *ownedDot) JawsUpdate(*jaws.Element) {}
@@ -324,30 +359,35 @@ func TestTemplate_RenderClosingWriteFailureDeletesOwnedElements(t *testing.T) {
}
}
-// TestRequestWriter_NewUIElementRenderedFailureDeletesOwnedElements checks that a
-// failing element-rendered hook unregisters the Element and everything it owns.
-func TestRequestWriter_NewUIElementRenderedFailureDeletesOwnedElements(t *testing.T) {
+// TestRequestWriter_NewUIReportsElementBeforeRendering checks that NewUI reports the
+// Element it created even when the render then fails, and unregisters it regardless.
+// Reporting at creation is what lets an owner reclaim an Element that never rendered.
+func TestRequestWriter_NewUIReportsElementBeforeRendering(t *testing.T) {
_, rq := newOwnedRequest(t)
- hookErr := errors.New("hook refused the element")
- var seen int
+ var seen []*jaws.Element
var sb strings.Builder
rw := RequestWriter{
Request: rq,
Writer: &sb,
- elementRendered: func(elem *jaws.Element) error {
- seen++
- return hookErr
+ elementCreated: func(elem *jaws.Element) {
+ seen = append(seen, elem)
},
}
- if err := rw.Template("div", "owned-parent", &ownedDot{}); !errors.Is(err, hookErr) {
- t.Fatalf("NewUI error = %v, want %v", err, hookErr)
+ // The template renders nested UI and then fails, so this writer creates one
+ // Element (the wrapper) and the nested writer inside it creates its own.
+ dot := &ownedDot{fail: errOwnedDotCheck}
+ if err := rw.Template("div", "owned-failafter", dot); !errors.Is(err, errOwnedDotCheck) {
+ t.Fatalf("render error = %v, want %v", err, errOwnedDotCheck)
+ }
+ if len(seen) != 1 {
+ t.Fatalf("hook calls = %d, want 1 (only Elements created directly through this writer)", len(seen))
}
- if seen != 1 {
- t.Fatalf("hook calls = %d, want 1 (only Elements created through this writer)", seen)
+ if !seen[0].Deleted() {
+ t.Error("the reported Element was not unregistered after its render failed")
}
if got := countRegistered(t, rq); got != 0 {
- t.Fatalf("registered elements after hook failure = %d, want 0", got)
+ t.Fatalf("registered elements after failed render = %d, want 0", got)
}
}
@@ -459,43 +499,43 @@ func TestPageTemplate_RenderFailureDeletesOwnedElements(t *testing.T) {
}
}
-// TestTemplate_UpdateDoesNotTrackRegisterOrRadioGroup pins the documented
-// exclusion: Register and RadioGroup create their Elements through
-// Request.NewElement rather than RequestWriter.NewUI, so a Template does not own
-// them and each update registers another set.
+// TestTemplate_UpdateTracksRegisterAndRadioGroup covers the two helpers that create
+// their Elements through Request.NewElement rather than RequestWriter.NewUI: they
+// report them to the writer's owner, so the counts stay flat across updates with no
+// client attached to acknowledge DOM removals.
//
-// The growth measured here is what the server does on its own. In a live session the
-// browser reports the ids it removes as the wrapper's new content is applied and the
-// Request unregisters those Elements; there is no client here to send that
-// acknowledgement, which is also the state of an Element whose id never reaches the
-// DOM. Should either helper start reporting through the element-rendered hook, these
-// counts become constant — update this test along with the Template, Register,
-// RadioGroup, README and skill docs stating the exclusion.
-func TestTemplate_UpdateDoesNotTrackRegisterOrRadioGroup(t *testing.T) {
- radios := named.NewBoolArray(false)
- radios.Add("1", "one")
+// The label-only case is why ownership is recorded at creation: RadioElement.Label
+// creates the radio Element for its for= attribute without ever rendering it, so an
+// after-render notification would miss it and it would accumulate.
+func TestTemplate_UpdateTracksRegisterAndRadioGroup(t *testing.T) {
+ newRadios := func() *named.BoolArray {
+ nba := named.NewBoolArray(false)
+ nba.Add("1", "one")
+ return nba
+ }
for _, tt := range []struct {
name string
template string
dot *ownedDot
- perRound int // Elements the helper adds per execution
+ perRound int // Elements the helper creates per execution
}{
{"register", "owned-register", &ownedDot{}, 1},
- {"radiogroup", "owned-radiogroup", &ownedDot{radios: radios}, 2}, // input and label
+ {"register discarded jid", "owned-register-discarded", &ownedDot{}, 1},
+ {"radiogroup", "owned-radiogroup", &ownedDot{radios: newRadios()}, 2}, // input and label
+ {"radiogroup label only", "owned-labelonly", &ownedDot{radios: newRadios()}, 2}, // unrendered radio, label
} {
t.Run(tt.name, func(t *testing.T) {
_, rq := newOwnedRequest(t)
tmpl := NewTemplate("div", tt.template, tt.dot)
elem := renderOwned(t, rq, tmpl)
- want := 1 + tt.perRound // the wrapper, plus the first execution's
+ want := 1 + tt.perRound // the wrapper, plus this execution's
if got := countRegistered(t, rq); got != want {
t.Fatalf("registered elements after render = %d, want %d", got, want)
}
for round := 1; round <= 3; round++ {
tmpl.JawsUpdate(elem)
- want += tt.perRound
if got := countRegistered(t, rq); got != want {
t.Fatalf("registered elements after %d update(s) = %d, want %d", round, got, want)
}
@@ -504,6 +544,125 @@ func TestTemplate_UpdateDoesNotTrackRegisterOrRadioGroup(t *testing.T) {
}
}
+// TestTemplate_UpdateFailureKeepsRegisterAndRadioGroupGeneration covers the remaining
+// leak mode: execution failing after either helper has already created its Elements.
+// The failed execution's output is discarded, so its Elements must go and the previous
+// generation must stay live to match the unchanged DOM.
+func TestTemplate_UpdateFailureKeepsRegisterAndRadioGroupGeneration(t *testing.T) {
+ newRadios := func() *named.BoolArray {
+ nba := named.NewBoolArray(false)
+ nba.Add("1", "one")
+ return nba
+ }
+
+ for _, tt := range []struct {
+ name string
+ template string
+ dot *ownedDot
+ perRound int
+ }{
+ {"register", "owned-register-failafter", &ownedDot{}, 1},
+ {"radiogroup", "owned-radiogroup-failafter", &ownedDot{radios: newRadios()}, 2},
+ } {
+ t.Run(tt.name, func(t *testing.T) {
+ jw, rq := newOwnedRequest(t)
+ logger := new(templateLogger)
+ jw.Logger = logger
+
+ tmpl := NewTemplate("div", tt.template, tt.dot)
+ elem := renderOwned(t, rq, tmpl)
+ want := 1 + tt.perRound
+ if got := countRegistered(t, rq); got != want {
+ t.Fatalf("registered elements after render = %d, want %d", got, want)
+ }
+ first := registeredJids(t, rq)
+
+ tt.dot.setFail(errOwnedDotCheck)
+ tmpl.JawsUpdate(elem)
+ if len(logger.errors) != 1 || !errors.Is(logger.errors[0], errOwnedDotCheck) {
+ t.Fatalf("logged errors = %v, want one %v", logger.errors, errOwnedDotCheck)
+ }
+ if got := registeredJids(t, rq); !slices.Equal(got, first) {
+ t.Fatalf("registered jids after failed update = %v, want the previous generation %v", got, first)
+ }
+
+ // The previous generation must still be owned, so the next success reclaims
+ // it rather than leaking it.
+ tt.dot.setFail(nil)
+ tmpl.JawsUpdate(elem)
+ if got := countRegistered(t, rq); got != want {
+ t.Fatalf("registered elements after recovery = %d, want %d", got, want)
+ }
+ if got := registeredJids(t, rq); slices.Equal(got, first) {
+ t.Fatal("the recovering update did not replace the previous generation")
+ }
+ })
+ }
+}
+
+// TestRadioGroup_OwnedByTheTemplateThatCalledIt pins where ownership is attributed
+// when a group's markup lands in a different wrapper than the RadioGroup call: the
+// outer template calls $.RadioGroup and passes the RadioElement values to a nested
+// wrapped template through its dot.
+//
+// Updating the outer template alone would not distinguish the two candidates, since
+// the cleanup walk recurses into the nested Template's own owned set either way. The
+// discriminating step is updating the nested Element on its own: with the radios no
+// longer rendered and no client to acknowledge the DOM removal, they must still be
+// registered, which is what fails if the nested template owned them.
+func TestRadioGroup_OwnedByTheTemplateThatCalledIt(t *testing.T) {
+ _, rq := newOwnedRequest(t)
+
+ radios := named.NewBoolArray(false)
+ radios.Add("1", "one")
+ box := &ownedRadioBox{show: true}
+ dot := &ownedDot{radios: radios, box: box}
+ outer := NewTemplate("div", "owned-radio-outer", dot)
+ outerElem := renderOwned(t, rq, outer)
+
+ // outer wrapper, nested wrapper, radio, label
+ const want = 4
+ if got := countRegistered(t, rq); got != want {
+ t.Fatalf("registered elements after render = %d, want %d", got, want)
+ }
+ inner := rq.GetElements(box)
+ if len(inner) != 1 {
+ t.Fatalf("nested wrapper elements = %d, want 1", len(inner))
+ }
+
+ // The nested template updates on its own and stops rendering the group. Counting
+ // its executions keeps the assertion below from passing vacuously: the radios must
+ // survive an update that ran and dropped them, not one that never happened.
+ box.show = false
+ execsBefore := box.execs
+ inner[0].JawsUpdate()
+ if box.execs != execsBefore+1 {
+ t.Fatalf("nested template executions = %d, want %d: the nested update did not run",
+ box.execs, execsBefore+1)
+ }
+ if got := countRegistered(t, rq); got != want {
+ t.Fatalf("registered elements after the nested update = %d, want %d: the radio and label "+
+ "belong to the template that called RadioGroup, so the nested update must not reclaim them", got, want)
+ }
+
+ // The owner reclaims them, along with the nested wrapper it also owns.
+ outer.JawsUpdate(outerElem)
+ if got := countRegistered(t, rq); got != 2 {
+ t.Fatalf("registered elements after the outer update = %d, want 2 (both wrappers, no group)", got)
+ }
+}
+
+// registeredJids returns the Jids still registered in rq, in ascending order.
+func registeredJids(t *testing.T, rq *jaws.Request) (jids []jaws.Jid) {
+ t.Helper()
+ for jid := jaws.Jid(1); jid <= maxProbedJid; jid++ {
+ if rq.GetElementByJid(jid) != nil {
+ jids = append(jids, jid)
+ }
+ }
+ return
+}
+
// TestTemplate_UpdateReclaimsManyTaggedDescendants exercises the batched
// unregister with a generation large enough to span several tag entries.
func TestTemplate_UpdateReclaimsManyTaggedDescendants(t *testing.T) {