Skip to content

Fix class helpers splitting non-ASCII whitespace class names - #612

Merged
mna merged 2 commits into
PuerkitoBio:masterfrom
sergioperezcheco:fix/class-ascii-token-boundaries
Oct 5, 2026
Merged

mna merged 2 commits into
PuerkitoBio:masterfrom
sergioperezcheco:fix/class-ascii-token-boundaries

Conversation

@sergioperezcheco

Copy link
Copy Markdown
Contributor

Single class names containing non-ASCII whitespace (NBSP, EM SPACE,
NARROW NBSP) are broken by the class helpers since #552: AddClass
re-appends the name split into pieces as extra classes, RemoveClass
cannot remove it, and ToggleClass behaves wrongly, while HasClass and
the class-attribute normalization treat those characters as part of the
token.

Root cause

HTML class tokens are separated by ASCII whitespace only (WHATWG HTML
standard 2.3.7); any other character is part of the token. #552 changed
getClassesSlice from an ASCII replacer plus strings.Split to
strings.Fields, which also splits on Unicode whitespace. HasClass and
getClassesAndAttr still normalize with the ASCII-only
classTrimReplacer, so the tokenizers now disagree:

  • AddClass("a\u00a0b") on a fresh element stores a\u00a0b a b
  • RemoveClass("a\u00a0b") on a\u00a0b keep leaves keep (fails) and on a\u00a0b alone clears the whole attribute
  • ToggleClass adds a duplicate a b instead of toggling the name
  • setClasses also trimmed Unicode whitespace from attribute edges

Fix

Verification

  • New regressions (property_unicode_class_test.go): 35 subtests fail on
    master 0ac7142 and pass with this change; attribute round-trips are
    byte-for-byte.
  • go test -race ./... passes.
  • go vet ./..., go build ./..., gofmt, git diff --check pass.
  • Benchmarks (darwin/arm64, Go 1.26.4, -count=3 means vs master):
    AddClass 27279 -> 28776 ns/op (+5.5%), RemoveClass 28166 -> 29047
    ns/op (+3.1%); B/op and allocs/op unchanged. FieldsFunc keeps the
    allocation win that motivated perf: replace regex+Split with strings.Fields in getClassesSlice #552.

AI-assisted contribution, disclosed per the Hermes automation workflow;
DCO sign-off included.

HTML class tokens are separated by ASCII whitespace only (WHATWG HTML
2.3.7); any other character is part of the token. PuerkitoBio#552 switched
getClassesSlice to strings.Fields, which also splits on Unicode
whitespace, so a single class name containing NBSP/EM SPACE/NARROW NBSP
was read as several tokens by Add/Remove/ToggleClass while HasClass and
the class-attribute normalization treat those characters as token
content. The result: AddClass re-appends the split pieces as new classes,
RemoveClass and ToggleClass cannot remove the name, and setClasses
trimmed Unicode whitespace from attribute edges.

Restore ASCII-only boundaries: getClassesSlice uses FieldsFunc over the
five ASCII whitespace characters, and the entry/exit trimming in the
helpers and setClasses trims the same set so Unicode whitespace at
attribute edges is preserved verbatim. Form feed stays a separator, and
the tokenizer change keeps the no-empty-entries behavior of PuerkitoBio#552.

Benchmarks (darwin/arm64, Go 1.26.4, -count=3 means vs master):
AddClass 27279 -> 28776 ns/op (+5.5%), RemoveClass 28166 -> 29047
ns/op (+3.1%); B/op and allocs/op unchanged (7024/1776 B, 374 allocs).

Signed-off-by: sergioperezcheco <checo520@outlook.com>
Cover Add/Remove/ToggleClass over class names containing NBSP, EM SPACE,
NARROW NBSP and vertical tab, mid-name and at attribute edges, plus
direct tokenizer expectations that only the five ASCII whitespace
characters split tokens. Attribute round-trips verify the stored value
byte-for-byte. These cases fail on master after PuerkitoBio#552 and pass with the
preceding change.

Signed-off-by: sergioperezcheco <checo520@outlook.com>
@sergioperezcheco

Copy link
Copy Markdown
Contributor Author

The workflow run for head 36c04bf is marked action_required and has not started any jobs. Could a maintainer approve it so CI can run? The regression, race-test, and benchmark results are included in the PR description.

@mna
mna merged commit 8b55f38 into PuerkitoBio:master Oct 5, 2026
6 checks passed
@mna

mna commented Oct 5, 2026

Copy link
Copy Markdown
Member

Thanks for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants