diff --git a/broadcast.go b/broadcast.go index e0b41d69..ad990d6c 100644 --- a/broadcast.go +++ b/broadcast.go @@ -25,6 +25,13 @@ import ( // // All convenience helpers on [Jaws] that call Broadcast inherit this requirement. // +// A [wire.Message.What] of [what.Replace] or [what.Remove] is rejected (as +// [ErrReplaceNotBroadcastable] or [ErrRemoveNotBroadcastable]) via reportMisuse and +// nothing is sent: each mutates a specific element's node in a way the broadcast path +// cannot keep in sync with the server-side registry, stranding the matched [Element] +// values with no reachable DOM node. Use [Element.Replace], or [Jaws.Delete] / [Element.Remove], +// for the identity-preserving forms. +// // A nil [wire.Message.Dest] targets every active Request; a [key.Key] Dest targets // the active Request with that identity key, and a zero key is dropped. Any other // Dest is expanded into tags. Plain strings and [Jid] values are illegal tag @@ -35,6 +42,14 @@ import ( // set; with a Logger the error is logged and the message is sent to the // destinations that did expand. func (jw *Jaws) Broadcast(msg wire.Message) { + switch msg.What { + case what.Replace: + jw.reportMisuse(fmt.Errorf("jaws: Broadcast: %w; use Element.Replace", ErrReplaceNotBroadcastable)) + return + case what.Remove: + jw.reportMisuse(fmt.Errorf("jaws: Broadcast: %w; use Jaws.Delete or Element.Remove", ErrRemoveNotBroadcastable)) + return + } switch dest := msg.Dest.(type) { case nil: // send to all active requests case key.Key: // send to the active request with this identity key @@ -335,13 +350,6 @@ func (jw *Jaws) Insert(target any, childIndex int, html template.HTML) { jw.broadcastTo(target, what.Insert, strconv.Itoa(childIndex)+"\n"+string(html)) } -// Replace replaces HTML on all HTML elements matching target. -// -// html is trusted HTML, matching [Jaws.SetInner] and [Jaws.Append]. -func (jw *Jaws) Replace(target any, html template.HTML) { - jw.broadcastTo(target, what.Replace, string(html)) -} - // Delete removes the HTML element(s) matching target. func (jw *Jaws) Delete(target any) { jw.broadcastTo(target, what.Delete, "") diff --git a/errors.go b/errors.go index fd95ed84..778f4883 100644 --- a/errors.go +++ b/errors.go @@ -68,6 +68,29 @@ var ErrInvalidChildIndex = errors.New("invalid child index") // helpers report this via reportMisuse and send nothing. var ErrReservedAttribute = errors.New("reserved attribute") +// ErrReplaceNotBroadcastable indicates an attempt to broadcast a [what.Replace] command. +// +// Replace swaps the whole target node for new HTML, which must carry the +// [Element]'s own "id" so the server-side [Element] keeps a reachable DOM node +// (see [Element.Replace], which validates exactly that). A broadcast delivers one +// payload to every element matching [wire.Message.Dest], so no single payload can +// preserve each element's distinct id and the matched Elements would be stranded +// with no matching DOM node. [Jaws.Broadcast] reports this via reportMisuse and +// sends nothing; use [Element.Replace] for the per-element form. +var ErrReplaceNotBroadcastable = errors.New("what.Replace cannot be broadcast") + +// ErrRemoveNotBroadcastable indicates an attempt to broadcast a [what.Remove] command. +// +// Remove deletes the child node named by Data from the matched element and requires +// the child's server-side [Element] to be unregistered too (see [Element.Remove], +// which calls [Request.DeleteElement]; the client acknowledges only the removal of +// the child's descendants, never the child itself). A broadcast forwards the command +// verbatim without that registry cleanup, and its Data names one request's child, so +// matched child Elements would be stranded with no reachable DOM node. [Jaws.Broadcast] +// reports this via reportMisuse and sends nothing; use [Jaws.Delete] to remove matched +// elements, or [Element.Remove] for the per-element child form. +var ErrRemoveNotBroadcastable = errors.New("what.Remove cannot be broadcast") + // ErrJavascriptDisabled is returned when the noscript probe indicates JavaScript is disabled. var ErrJavascriptDisabled = errors.New("javascript is disabled") diff --git a/jaws_test.go b/jaws_test.go index c19a7b7a..70f7e9d4 100644 --- a/jaws_test.go +++ b/jaws_test.go @@ -1375,10 +1375,6 @@ func TestCoverage_GenerateHeadAndConvenienceBroadcasts(t *testing.T) { if msg := nextBroadcast(t, jw); msg.What != what.Insert || msg.Data != "0\na" { t.Fatalf("unexpected insert msg %#v", msg) } - jw.Replace(target, "b") - if msg := nextBroadcast(t, jw); msg.What != what.Replace || msg.Data != "b" { - t.Fatalf("unexpected replace msg %#v", msg) - } jw.Delete(target) if msg := nextBroadcast(t, jw); msg.What != what.Delete { t.Fatalf("unexpected delete msg %#v", msg) @@ -1569,6 +1565,61 @@ func TestJaws_AttrHelpersRejectReservedId(t *testing.T) { } } +func TestBroadcast_RejectsElementMutatingCommands(t *testing.T) { + // Replace and Remove mutate a specific element's node in a way the broadcast path + // cannot keep in sync with the server-side registry: a raw broadcast forwards the + // command verbatim to every matching element, stranding the server-side Element + // with no reachable DOM node (the #199 identity desync). Both must be rejected + // before reaching bcastCh, regardless of Data. + tests := []struct { + name string + what what.What + wantErr error + }{ + {"Replace", what.Replace, ErrReplaceNotBroadcastable}, + {"Remove", what.Remove, ErrRemoveNotBroadcastable}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + jw, err := New() + if err != nil { + t.Fatal(err) + } + defer jw.Close() + logger := &captureErrorLogger{} + jw.Logger = logger + + call := func() { + jw.Broadcast(wire.Message{ + Dest: tag.Tag("t"), + What: tt.what, + Data: "whatever", + }) + } + if deadlock.Debug { + func() { + defer func() { + if recover() == nil { + t.Fatalf("broadcast of %v did not panic in a debug build", tt.what) + } + }() + call() + }() + } else { + call() + } + if !errors.Is(logger.err, tt.wantErr) { + t.Fatalf("error = %v, want %v", logger.err, tt.wantErr) + } + select { + case msg := <-jw.bcastCh: + t.Fatalf("%v queued broadcast %#v", tt.what, msg) + default: + } + }) + } +} + func TestBroadcast_NoneDestination(t *testing.T) { jw, err := New() if err != nil { diff --git a/request_test.go b/request_test.go index 27d91742..1b700bb5 100644 --- a/request_test.go +++ b/request_test.go @@ -2540,25 +2540,6 @@ func TestRequest_IncomingRemoveDoesNotDeleteMessageJidListedInData(t *testing.T) }) } -func TestRequest_ReplaceMessageTargetsElementHTML(t *testing.T) { - rq := newTestRequest(t) - defer rq.Close() - - tagValue := &testUi{} - jid := rq.Register(tagValue) - html := template.HTML(`