Skip to content

fix: keep comment-like text inside attribute-selector strings#654

Open
spokodev wants to merge 1 commit into
adobe:mainfrom
spokodev:fix/comment-in-attribute-selector-string
Open

fix: keep comment-like text inside attribute-selector strings#654
spokodev wants to merge 1 commit into
adobe:mainfrom
spokodev:fix/comment-in-attribute-selector-string

Conversation

@spokodev

Copy link
Copy Markdown

The selector parser strips /* ... */ from the entire selector string before splitting on commas, without checking for quoted strings. A /* that appears inside a quoted attribute value is literal string content, not a comment, so the value is silently destroyed:

const { parse } = require("@adobe/css-tools");

parse('a[title="/*x*/"]{color:red}').stylesheet.rules[0].selectors;
// expected: [ 'a[title="/*x*/"]' ]
// actual:   [ 'a[title=""]' ]        <- attribute value destroyed

parse('a[data="a/*b*/c"]{color:red}').stylesheet.rules[0].selectors;
// expected: [ 'a[data="a/*b*/c"]' ]
// actual:   [ 'a[data="ac"]' ]       <- value spliced, meaning changed

This is silent data corruption: the AST holds a different selector than the input, so stringify(parse(s)) emits a rule that no longer matches the same elements.

Spec

CSS Syntax Level 3 recognizes comment tokens only at the top level of tokenization. Once a string token has begun (after " or '), /* does not start a comment, so [title="/*x*/"] has the literal attribute value /*x*/.

Fix

Add removeCommentWithQuoteSupport to the existing stringSearch utility, mirroring the quote-aware scanning already used by indexOfArrayWithBracketAndQuoteSupport (it reuses indexOfArrayNonEscaped to find the matching close quote and respects backslash escapes). selector() now calls it instead of the naive commentRegex replace.

Real comments outside strings are still stripped, so existing behavior is preserved:

The declaration-value comment strip (declaration()) is left untouched.

Tests

Added a describe block in test/parse.test.ts covering the corrupted cases (double-quoted, single-quoted, spliced, real-comment-adjacent) plus guard cases that assert real comments are still removed. Full suite: 228 passing, lint clean.

The selector parser stripped `/* ... */` from the whole selector string
before splitting on commas, ignoring quoted strings. A `/*` inside a
quoted attribute value is literal string content, not a comment, so this
silently destroyed the value:

  parse('a[title="/*x*/"]{color:red}').stylesheet.rules[0].selectors
  // was: ['a[title=""]']  -> now: ['a[title="/*x*/"]']

  parse('a[data="a/*b*/c"]{color:red}').stylesheet.rules[0].selectors
  // was: ['a[data="ac"]'] -> now: ['a[data="a/*b*/c"]']

Per CSS Syntax Level 3, comment tokens are only recognized at the top
level; once a string token begins, `/*` does not start a comment.

Add removeCommentWithQuoteSupport to the existing stringSearch utility,
mirroring its quote-aware scanning, and use it in selector() instead of
the naive regex replace. Real comments outside strings are still removed
(a:not(/*x*/.b) -> a:not(.b), a /*c*/ b -> a  b). The declaration-value
comment strip is left unchanged.
@spokodev spokodev force-pushed the fix/comment-in-attribute-selector-string branch from 5ee5c08 to f7b8cd8 Compare June 24, 2026 10:43
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