fix: escape a lone hyphen in identifier mode (CSSOM serialize)#37
Open
spokodev wants to merge 1 commit into
Open
fix: escape a lone hyphen in identifier mode (CSSOM serialize)#37spokodev wants to merge 1 commit into
spokodev wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
In identifier mode, a lone
-is returned unescaped:A bare
-is not a valid CSS<ident-token>, so this output does not round-trip and produces an invalid selector when used as a class/ID body.Why it is wrong
The CSSOM "serialize an identifier" algorithm (the one
CSS.escape()implements) has an explicit rule: if the character is the first character and is a-(U+002D), and there is no second character, then escape it. So the reference output is\-:The CSS Syntax Level 3 "would start an identifier" check agrees: a
-only starts an identifier when the next code point is a name-start code point, another-, or a valid escape. A lone-does not start an<ident-token>at all.Round-trip evidence with the
css-treeparser:The existing
if (isIdentifier)block already special-cases the leading-followed by-or a digit (--foo,-0foo) to stay spec-compliant. The lone-is the one remaining case it misses.The fix
One branch in the
isIdentifierpost-processing block: when the whole output is a single-, escape it to\-.String mode is unchanged:
cssesc('-')still returns-, which is fine in a string context. Other inputs (--,-1,-a,--foo) are unchanged. Bothsrc/cssesc.js(template source) and the builtcssesc.jsare updated, andnpm run buildreproduces the built file byte-for-byte.Tests
Added two assertions next to the existing leading-hyphen cases:
cssesc('-', { isIdentifier: false })stays-, andcssesc('-', { isIdentifier: true })is now\-. The new identifier assertion fails on the current code and passes with the fix; the full suite stays green.