From 048bc98ddf69f3ff41a69e40e7e8421be85de8ab Mon Sep 17 00:00:00 2001 From: dualfroz Date: Fri, 4 Sep 2026 07:52:26 +0200 Subject: [PATCH] fix: remove every occurrence of a repeated class RemoveClass deleted each class with one strings.ReplaceAll pass over the space-wrapped class list. Two adjacent occurrences share the space between them, so " a a " holds only one non-overlapping " a " and the second survived: RemoveClass("a") on class="a a" returned with HasClass("a") still true. ToggleClass used the same expression and so toggled such a class the wrong way. Replace one occurrence at a time, re-scanning from the start, in a helper shared by both call sites. Whitespace between the remaining classes is untouched. --- property.go | 26 +++++++++++++++++---- property_test.go | 59 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 4 deletions(-) diff --git a/property.go b/property.go index bda736a..5c8df86 100644 --- a/property.go +++ b/property.go @@ -167,7 +167,7 @@ func (s *Selection) RemoveClass(class ...string) *Selection { } else { classes, attr := getClassesAndAttr(n) for _, rcl := range rclasses { - classes = strings.ReplaceAll(classes, " "+rcl+" ", " ") + classes = removeClassFromClasses(classes, rcl) } setClasses(n, attr, classes) @@ -191,9 +191,8 @@ func (s *Selection) ToggleClass(class ...string) *Selection { for _, n := range s.Nodes { classes, attr := getClassesAndAttr(n) for _, tcl := range tcls { - spaceAroundTcl := " " + tcl + " " - if strings.Contains(classes, spaceAroundTcl) { - classes = strings.ReplaceAll(classes, spaceAroundTcl, " ") + if strings.Contains(classes, " "+tcl+" ") { + classes = removeClassFromClasses(classes, tcl) } else { classes += tcl + " " } @@ -205,6 +204,25 @@ func (s *Selection) ToggleClass(class ...string) *Selection { return s } +// removeClassFromClasses removes every occurrence of cl from the normalized +// class string (a class list wrapped in spaces, as returned by +// getClassesAndAttr). +// +// A single strings.ReplaceAll pass is not enough: two adjacent occurrences +// share the space between them, so " a a " contains only one non-overlapping +// " a " and the second one survives the replacement. Replacing one at a time +// re-scans from the start and reaches it. +func removeClassFromClasses(classes, cl string) string { + target := " " + cl + " " + for { + replaced := strings.Replace(classes, target, " ", 1) + if replaced == classes { + return classes + } + classes = replaced + } +} + func getAttributePtr(attrName string, n *html.Node) *html.Attribute { if n == nil { return nil diff --git a/property_test.go b/property_test.go index 1095dcc..e74040b 100644 --- a/property_test.go +++ b/property_test.go @@ -250,3 +250,62 @@ func TestToggleClass(t *testing.T) { t.Errorf("Expected #nf1 to have no classes, have %q", a) } } + +func TestRemoveClassRepeatedInSource(t *testing.T) { + cases := []struct { + class string + remove string + expected string + }{ + {"a a", "a", ""}, + {"a a a", "a", ""}, + {"a b a", "a", "b"}, + {"a b a b", "a", "b b"}, + {"a b a b", "a b", ""}, + {"a a b", "a", "b"}, + {"b a a", "a", "b"}, + } + + for _, c := range cases { + doc, err := NewDocumentFromReader(strings.NewReader(`
`)) + if err != nil { + t.Fatal(err) + } + + sel := doc.Find("#t") + sel.RemoveClass(c.remove) + + got, _ := sel.Attr("class") + if got != c.expected { + t.Errorf("class=%q RemoveClass(%q): got class %q, want %q", c.class, c.remove, got, c.expected) + } + + for _, removed := range strings.Fields(c.remove) { + if sel.HasClass(removed) { + t.Errorf("class=%q RemoveClass(%q): still has class %q", c.class, c.remove, removed) + } + } + } +} + +func TestToggleClassRepeatedInSource(t *testing.T) { + doc, err := NewDocumentFromReader(strings.NewReader(`
`)) + if err != nil { + t.Fatal(err) + } + + sel := doc.Find("#t") + + sel.ToggleClass("a") + if sel.HasClass("a") { + t.Error("expected #t to not have class a after toggling it off") + } + if !sel.HasClass("b") { + t.Error("expected #t to keep class b") + } + + sel.ToggleClass("a") + if !sel.HasClass("a") { + t.Error("expected #t to have class a after toggling it back on") + } +}