Skip to content

fix: keep a trailing slash in an unquoted attribute value - #330

Open
OsamaAnsar wants to merge 1 commit into
taoqf:mainfrom
OsamaAnsar:fix/unquoted-attribute-value-trailing-slash
Open

OsamaAnsar wants to merge 1 commit into
taoqf:mainfrom
OsamaAnsar:fix/unquoted-attribute-value-trailing-slash

Conversation

@OsamaAnsar

Copy link
Copy Markdown

A / right before > after an unquoted attribute value is currently taken as a self-closing slash. The slash is dropped from the value and the element is closed immediately, so its content ends up outside it:

parse('<a href=/foo/>x</a>').toString()
// before: '<a href=/foo></a>x'      attrs: { href: '/foo' }
// after:  '<a href=/foo/>x</a>'     attrs: { href: '/foo/' }

parse('<p><a href=http://example.com/>Example</a> and <a href=/docs/>docs</a></p>').toString()
// before: '<p><a href=http://example.com></a>Example and <a href=/docs></a>docs</p>'
// after:  '<p><a href=http://example.com/>Example</a> and <a href=/docs/>docs</a></p>'

<a href=/foo/ >x</a> (space before >) already worked.

Reading of the spec: in the "attribute value (unquoted) state" (13.2.5.38) only whitespace and > end the value; / falls under "Anything else" and is appended to the value. So <a href=/foo/> is an a with href="/foo/" that is not self-closing.

Fix: the tag regex captures (\/?)> as the closing slash. After destructuring, if that slash directly follows an unquoted value (name=value with nothing after it) it is moved back onto attributes. This only runs when a closing slash was captured and is a single anchored regex test.

Judgment call you may want to weigh in on: void elements are deliberately left as they were. <img src=a.png/> is still a self-closed img with src="a.png", because (a) a void element never has content so nothing is lost structurally, and (b) test/tests/issues/207.js (<foo attr=bar/> with voidTag.tags: ['foo']) relies on it; applying the spec reading there as well fails that test ('<foo attr=bar/ />' vs the expected '<foo attr=bar />'), and I did not want to change an existing test. If you would rather apply the spec reading to void elements too, it is just dropping the !voidTag.isVoidElement(tagName) condition plus updating that test.

Other cases, unchanged by this PR: <a href="/foo/"/> (quoted), <custom-el disabled/> (no value), <a href=/foo /> (space before the slash is still a self-closing start tag).

Tests: new test/tests/unquoted-attribute-slash.js with 7 tests. With only the src/ change reverted, 4 of them fail (the 3 others are guards for the unchanged cases above); with the change they pass.

How I ran things: bun is not installed here, so I bundled src/index.ts with esbuild (css-select 5.2.2, entities 8.0.0 as in bun.lock) and ran mocha on Node 22 against that bundle, skipping issues/280.js (needs the published npm packages). main: 303 passing, 4 failing. This branch: 310 passing, same 4 failing. Those 4 are artifacts of my setup (the two Module Import tests and two unclosed tag repair subprocess tests need ts-node / a built dist). tsc --noEmit is clean.

Not tested: bun run build / test:dist against a real dist, eslint, Node 24/26, benchmarks.

In '<a href=/foo/>x</a>' the tag regex captured the final '/' as a
self-closing slash, so the value became '/foo' and the element was closed
immediately, moving its content out of it ('<a href=/foo></a>x').

In the HTML tokenizer's unquoted attribute value state '/' is an ordinary
value character and only whitespace or '>' ends the value, so the slash
belongs to the value. Treat it that way for non-void elements.

Void elements keep their lenient behaviour ('<img src=a.png/>' is still a
self-closed tag with src 'a.png'); they never have content, and
test/tests/issues/207.js relies on it.
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.

1 participant