Skip to content

fix: align CSS escape consumption with the spec#1791

Open
ursm wants to merge 1 commit into
fb55:masterfrom
ursm:fix/css-escape-consumption
Open

fix: align CSS escape consumption with the spec#1791
ursm wants to merge 1 commit into
fb55:masterfrom
ursm:fix/css-escape-consumption

Conversation

@ursm

@ursm ursm commented Jun 2, 2026

Copy link
Copy Markdown

Summary

funescape and the identifier tokenizer (reName/reEscape) diverged from CSS Syntax Level 3 — "consume an escaped code point" in four ways. Each is fixed here, with regression tests.

1. Uppercase hex digits in identifiers

reName lacked the i flag, so a hex escape with uppercase letters was under-consumed. For example #hel\6C o:

  • \6C only matched \6 (since C isn't in [\da-f] without i), leaving C as a name char and the trailing space as a descendant combinator → parsed as id hell + descendant o.
  • With i, \6C is consumed as a two-digit hex escape and the following space is consumed as the escape's trailing whitespace → id hello, as intended.

reEscape already had the i flag, so this only needed to be mirrored on reName.

2. CRLF after a hex escape

The spec consumes a single whitespace after a hex escape, and treats a CRLF pair as one whitespace. \s? consumed only one of the two characters, leaving a stray \r/\n in the identifier. Replaced with (?:\r\n|\s)? in both regexes.

3. Trailing backslash at EOF

A backslash at EOF is a parse error that yields U+FFFD per spec. Previously it was dropped (or threw, depending on the call path). Added a |$ alternative to both regexes and an escaped === "" guard in funescape.

4. Surrogate code points → U+FFFD

The spec maps U+0000, surrogates, and out-of-range values to U+FFFD. The existing check handled U+0000 and > 0x10FFFF, but String.fromCodePoint accepts surrogate values and returns lone surrogates, so escapes like \D800 leaked through. Added the surrogate range to the check.

Tests

Added fixtures in src/__fixtures__/tests.ts covering each case (uppercase hex, CRLF consumption, trailing-backslash → U+FFFD, surrogate → U+FFFD). All existing tests continue to pass.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests

    • Added test cases for CSS selector parsing edge cases, including escape sequence handling and end-of-input behavior.
  • Bug Fixes

    • Enhanced CSS identifier parsing to properly handle invalid escape sequences, surrogate characters, and incomplete escapes at end-of-input.

The selector tokenizer and `funescape` diverged from CSS Syntax Level 3's
"consume an escaped code point" in four ways:

- Uppercase hex digits in an identifier (e.g. `#hel\6C o`) were mis-tokenized:
  `reName` lacked the `i` flag, so `\6C` only consumed `\6`, leaving `C` as a
  name char and the following space as a combinator. Added `i` to `reName`
  (`reEscape` already had it).
- A `CRLF` after a hex escape was only half-consumed: `\s?` ate one of the two
  characters. Replaced with `(?:\r\n|\s)?` in both regexes so a CRLF pair is
  consumed as a single whitespace.
- A trailing backslash at EOF (an empty escape) now resolves to U+FFFD instead
  of being dropped/throwing. Added `|$` to both regexes and an `escaped === ""`
  guard in `funescape`.
- Surrogate code points (U+D800–U+DFFF) now resolve to U+FFFD. `fromCodePoint`
  accepts surrogate values and returns lone surrogates, so they previously
  leaked through.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: fe9ff0c0-76e0-4e55-970f-1e1d728d3121

📥 Commits

Reviewing files that changed from the base of the PR and between b365f01 and 1e4c586.

📒 Files selected for processing (2)
  • src/__fixtures__/tests.ts
  • src/parse.ts

📝 Walkthrough

Walkthrough

This PR strengthens CSS selector parsing by refining escape sequence recognition and unescaping logic. Escape regex patterns are updated to handle broader forms, and the unescaping function now explicitly rejects surrogate code points and empty escapes at end-of-input, mapping them to U+FFFD. Four test cases validate these edge cases.

Changes

CSS Escape Sequence Parsing

Layer / File(s) Summary
Escape sequence recognition and unescaping
src/parse.ts
reName and reEscape regex patterns updated to recognize broader escape forms and CRLF variants. funescape now handles empty escape at end-of-input and rejects surrogate code points (U+D800–U+DFFF) as U+FFFD.
Test cases for escape edge cases
src/__fixtures__/tests.ts
Four new test cases validate surrogate escape handling, uppercase hex escape decoding, CRLF consumption after hex escapes, and trailing backslash at EOF replacement with U+FFFD.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Escapes now dance with proper grace,
Surrogates find their rightful place,
U+FFFD replaces what can't be,
And trailing backslashes fly free!
✨ CSS parsing, now complete!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: aligning CSS escape consumption with the specification, which is the core objective addressed across both modified files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

src/__fixtures__/tests.ts

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.

src/parse.ts

ESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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