From 43d43df86747300bc0f3cb38784d11418a2922ad Mon Sep 17 00:00:00 2001 From: AdamMagued Date: Fri, 2 Oct 2026 15:22:07 +0000 Subject: [PATCH] fix: account for form ancestor context in HTML fragment cache golang.org/x/net/html.ParseFragmentWithOptions initializes its form pointer based on whether the context element or any of its ancestors is a form element. If the context has a form ancestor, nested form tags are discarded per HTML parsing rules. Previously, cachedParseHtmlWithContext keyed cached fragments strictly by context tag name. When a selection contained targets both inside and outside forms sharing the same tag name, the first visited target cached its parsed fragment, leading to incorrect parsing for subsequent targets under the other form-ancestor state. Differentiate the fragment cache key by appending a form-ancestor indicator when context or any of its ancestors is a form element. Fixes #614 --- manipulation.go | 20 +++++++- manipulation_test.go | 120 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 138 insertions(+), 2 deletions(-) diff --git a/manipulation.go b/manipulation.go index d9edcd5..ce7731d 100644 --- a/manipulation.go +++ b/manipulation.go @@ -5,6 +5,7 @@ import ( "strings" "golang.org/x/net/html" + "golang.org/x/net/html/atom" ) // After applies the selector from the root document and inserts the matched elements @@ -677,11 +678,26 @@ func (s *Selection) eachNodeHtml(htmlStr string, isParent bool, mergeFn func(n * return s } +// hasFormAncestor reports whether n or any of its ancestors is an HTML form element. +// This mirrors the logic in net/html.ParseFragmentWithOptions, which initializes its +// form pointer from context and its ancestor chain. +func hasFormAncestor(n *html.Node) bool { + for ; n != nil; n = n.Parent { + if n.Type == html.ElementNode && (n.DataAtom == atom.Form || n.Data == "form") { + return true + } + } + return false +} + // cachedParseHtmlWithContext returns parseHtmlWithContext(htmlStr, context), reusing a prior -// result when context's nodeName has already been seen. Callers pass their own -// cache map so the cache lifetime matches the caller's loop scope. +// result when context's nodeName has already been seen with the same form ancestor state. +// Callers pass their own cache map so the cache lifetime matches the caller's loop scope. func cachedParseHtmlWithContext(cache map[string][]*html.Node, htmlStr string, context *html.Node, opts []html.ParseOption) []*html.Node { key := nodeName(context) + if hasFormAncestor(context) { + key += ":form" + } if nodes, ok := cache[key]; ok { return nodes } diff --git a/manipulation_test.go b/manipulation_test.go index fd5e020..cb5145b 100644 --- a/manipulation_test.go +++ b/manipulation_test.go @@ -759,3 +759,123 @@ func TestHtmlWithNonElementNode(t *testing.T) { }) } } + +func TestFormAncestorCache(t *testing.T) { + const htmlTemplate = `` + + `
c1
` + + `
c2
` + + `` + + const insertFragment = `
` + const wrapFragment = `
` + + cases := []struct { + name string + fragment string + fn func(*Selection, string) *Selection + }{ + {"AppendHtml", insertFragment, (*Selection).AppendHtml}, + {"PrependHtml", insertFragment, (*Selection).PrependHtml}, + {"SetHtml", insertFragment, (*Selection).SetHtml}, + {"BeforeHtml", insertFragment, (*Selection).BeforeHtml}, + {"AfterHtml", insertFragment, (*Selection).AfterHtml}, + {"ReplaceWithHtml", insertFragment, (*Selection).ReplaceWithHtml}, + {"WrapHtml", wrapFragment, (*Selection).WrapHtml}, + {"WrapInnerHtml", wrapFragment, (*Selection).WrapInnerHtml}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + // Control: apply to target-out only + docCtrlOut := loadString(t, htmlTemplate) + tc.fn(docCtrlOut.Find("#target-out"), tc.fragment) + ctrlOutHTML, err := docCtrlOut.Find("#p-out").Html() + if err != nil { + t.Fatal(err) + } + + // Control: apply to target-in only + docCtrlIn := loadString(t, htmlTemplate) + tc.fn(docCtrlIn.Find("#target-in"), tc.fragment) + ctrlInHTML, err := docCtrlIn.Find("#p-in").Html() + if err != nil { + t.Fatal(err) + } + + // Batch 1: outside element first [target-out, target-in] + docOutFirst := loadString(t, htmlTemplate) + selOutFirst := docOutFirst.Find("#target-out").AddNodes(docOutFirst.Find("#target-in").Get(0)) + tc.fn(selOutFirst, tc.fragment) + outFirst_OutHTML, _ := docOutFirst.Find("#p-out").Html() + outFirst_InHTML, _ := docOutFirst.Find("#p-in").Html() + + if outFirst_OutHTML != ctrlOutHTML { + t.Errorf("outside-first target-out: got %q, want %q", outFirst_OutHTML, ctrlOutHTML) + } + if outFirst_InHTML != ctrlInHTML { + t.Errorf("outside-first target-in: got %q, want %q", outFirst_InHTML, ctrlInHTML) + } + + // Batch 2: inside element first [target-in, target-out] + docInFirst := loadString(t, htmlTemplate) + selInFirst := docInFirst.Find("#target-in").AddNodes(docInFirst.Find("#target-out").Get(0)) + tc.fn(selInFirst, tc.fragment) + inFirst_OutHTML, _ := docInFirst.Find("#p-out").Html() + inFirst_InHTML, _ := docInFirst.Find("#p-in").Html() + + if inFirst_OutHTML != ctrlOutHTML { + t.Errorf("inside-first target-out: got %q, want %q", inFirst_OutHTML, ctrlOutHTML) + } + if inFirst_InHTML != ctrlInHTML { + t.Errorf("inside-first target-in: got %q, want %q", inFirst_InHTML, ctrlInHTML) + } + + // Empty selection control + docEmpty := loadString(t, htmlTemplate) + emptySel := docEmpty.Find(".nonexistent") + tc.fn(emptySel, tc.fragment) + }) + } +} + +func TestAppendHtmlFormAncestorContext(t *testing.T) { + doc := loadString(t, ` +
+
+ `) + + doc.Find(".target").AppendHtml(`
`) + + assertLength(t, doc.Find("#t-out form").Nodes, 1) + assertLength(t, doc.Find("#t-out form input").Nodes, 1) + assertLength(t, doc.Find("#t-in form").Nodes, 0) + assertLength(t, doc.Find("#t-in input").Nodes, 1) +} + +func TestPrependHtmlFormAncestorContext(t *testing.T) { + doc := loadString(t, ` +

existing

+

existing

+ `) + + doc.Find(".target").PrependHtml(`
`) + + assertLength(t, doc.Find("#t-out form").Nodes, 1) + assertLength(t, doc.Find("#t-out form input").Nodes, 1) + assertLength(t, doc.Find("#t-in form").Nodes, 0) + assertLength(t, doc.Find("#t-in input").Nodes, 1) +} + +func TestSetHtmlFormAncestorContext(t *testing.T) { + doc := loadString(t, ` +

old

+

old

+ `) + + doc.Find(".target").SetHtml(`
`) + + assertLength(t, doc.Find("#t-out form").Nodes, 1) + assertLength(t, doc.Find("#t-out form input").Nodes, 1) + assertLength(t, doc.Find("#t-in form").Nodes, 0) + assertLength(t, doc.Find("#t-in input").Nodes, 1) +}