Skip to content

Fix the forgiving fallback and EOF-terminated arguments - #179

Merged
dperini merged 1 commit into
dperini:masterfrom
jdalton:fix/forgiving-and-eof
Sep 5, 2026
Merged

dperini merged 1 commit into
dperini:masterfrom
jdalton:fix/forgiving-and-eof

Conversation

@jdalton

@jdalton jdalton commented Sep 4, 2026 •

Copy link
Copy Markdown
Collaborator

Two parse defects from the 2.2.25 compiler rework (7a22775): the forgiving-selector fallback tests a doubled escape that can never match, and three pattern groups lost their EOF terminator. Fixing them needs the argument of a logical pseudo-class delimited by a scan rather than a regular expression, since nesting is involved.

Detail, and how it was checked

Two parse defects from the 2.2.25 compiler rework (7a22775).

The forgiving-selector fallback tests /(:(?:is|where)\x28)/, where the doubled escape matches a literal backslash rather than an opening parenthesis, so an unsupported argument such as :not(:is(svg|div)) emits "unknown pseudo-class selector" instead of matching nothing.

The linguistic, logicalsel and treestruct groups lost their (?:\x29|$) terminator, so :not([class] and meta[charset="utf-8" are parse errors rather than being closed by EOF the way the CSS Syntax parser closes any open construct. That is /css/selectors/missing-right-token.html.

Restoring the terminator alone reintroduces the bug it had been papering over: with [^()]*|.* the greedy alternative swallows the closing parenthesis of a nested argument, so :not(:is(div)) compiles :is(div)). A regular expression cannot track nesting, so the argument of :is, :where, :matches, :not and :has is delimited by a scan for the balanced closing parenthesis, honoring quotes and escapes, and falling back to EOF. It returns a match-like array so the compile loop keeps popping the remainder as before.

One more change belongs with these, or the first fix trades a throw for a wrong answer: when the validator cannot read a selector that holds a forgiving list, parse() hands on the fragments the validator did match, each compiled as a selector of its own, so div:not(:is(svg|div)) matches every element in the document rather than the divs. It now hands on the selector, whose forgiving argument is already evaluated inside a try/catch.

Extracted from #167 as a standalone change: one file, applies to master on its own, and checked against a fixture to confirm it fixes what it describes and moves nothing else.

References: the spec, the browser source, and what each part was reasoned from

This patch applies to master on its own. The sixteen in this series were checked by cherry-picking them onto master one after another, in this order and in reverse, and all sixteen land without a conflict.

@jdalton
jdalton force-pushed the fix/forgiving-and-eof branch from 2d3da5d to 6eeb71e Compare September 4, 2026 18:00
Two parse defects from the 2.2.25 compiler rework (7a22775).
 The forgiving-selector fallback tests /(:(?:is|where)\\x28)/, where the doubled escape matches a literal backslash rather than an opening parenthesis, so an unsupported argument such as ':not(:is(svg|div))' emits "unknown pseudo-class selector" instead of matching nothing.
 The linguistic, logicalsel and treestruct groups lost their '(?:\x29|$)' terminator, so ':not([class]' and 'meta[charset="utf-8"' are parse errors rather than being closed by EOF the way the CSS Syntax parser closes any open construct. That is /css/selectors/missing-right-token.html.
 Restoring the terminator alone reintroduces the bug it had been papering over: with '[^()]*|.*' the greedy alternative swallows the closing parenthesis of a nested argument, so ':not(:is(div))' compiles ':is(div))'. A regular expression cannot track nesting, so the argument of :is, :where, :matches, :not and :has is delimited by a scan for the balanced closing parenthesis, honoring quotes and escapes, and falling back to EOF. It returns a match-like array so the compile loop keeps popping the remainder as before.
 One more change belongs with these, or the first fix trades a throw for a wrong answer: when the validator cannot read a selector that holds a forgiving list, parse() hands on the fragments the validator did match, each compiled as a selector of its own, so 'div:not(:is(svg|div))' matches every element in the document rather than the divs. It now hands on the selector, whose forgiving argument is already evaluated inside a try/catch.

References:

- Spec: https://drafts.csswg.org/selectors-4/#typedef-forgiving-selector-list — a forgiving selector list drops the items it cannot parse
- Spec: https://drafts.csswg.org/css-syntax/#consume-simple-block — a construct left open at EOF is closed rather than rejected
- Chromium: https://github.com/chromium/chromium/blob/155.0.8041.1/third_party/blink/renderer/core/css/selector_checker.cc#L2516 — ':is()' and ':where()' match any item in the list
- MDN: https://developer.mozilla.org/en-US/docs/Web/CSS/:is
@jdalton
jdalton force-pushed the fix/forgiving-and-eof branch from 6eeb71e to a959b4a Compare September 5, 2026 02:47
@jdalton

jdalton commented Sep 5, 2026 •

Copy link
Copy Markdown
Collaborator Author

The parser fixes are split by behavior:

Correction to my earlier integration claim: conflict-free cherry-picks are not sufficient validation, and the expanded series cannot be treated as order-independent. The later audit found an omitted declaration in #182, now corrected, and documented composition requirements for the legacy and optimization PRs. The integration checkpoint records those inputs and tests.

#198 separately guards a failed attribute match without changing valid end-of-input recovery.

@dperini

dperini commented Sep 5, 2026

Copy link
Copy Markdown
Owner

just a note...
A non terminated construct, referred here as just EOF, might in fact be any of:
EOL (line), EOT (text), EOD (document), EOF (file)
could generically be named EOI (input) applied to a selector string.

It actually force the parsing to stop because it can not proceed further.
However it can not be thought as an error.

@jdalton

jdalton commented Sep 5, 2026 •

Copy link
Copy Markdown
Collaborator Author

@dperini The four escaped line-continuation cases are now fixed in #198 at a6dbae3. Their TODO markers are gone, and all 163 attribute tests pass.

End of input and a newline have different rules.

The end of a selector string can close an unfinished construct. Inside a quoted string, a backslash followed by a newline continues the string, but a raw newline makes it invalid. #198 now handles LF, CR, CRLF and form feed without losing the boundaries of hexadecimal escapes.

Specification and validation details

CSS string tokenization defines string recovery at EOF, bad strings for raw newlines, and escaped line continuations. Function parsing returns the function at EOF. A specification-level parse error does not always require a thrown DOM SyntaxError when recovery is defined.

The compiler still guards a missing attribute match before reading match[0]. The parser now also consumes valid string continuations before normalizing whitespace. It preserves valid EOF recovery and rejects raw newlines, including form feed and a newline at the end of an unclosed string.

Validation: 163 attribute tests and four runtime tests pass with jsdom 26.1.0 and 30.0.1. All 155 browser cases agree with Chromium for select, first and match, including repeated calls. All 41 selected WPT pages pass without changing the baseline. There are no TODOs or skips in the attribute suite.

jdalton added a commit that referenced this pull request Sep 17, 2026
Consolidated commits:
- a959b4a Fix the forgiving fallback and EOF-terminated arguments
  Original author: John-David Dalton <john.david.dalton@gmail.com>
- 8a91824 Anchor the alternation in the :link and :placeholder-shown tests
  Original author: John-David Dalton <john.david.dalton@gmail.com>
- efa85cb Answer an id selector from the id map instead of walking
  Original author: John-David Dalton <john.david.dalton@gmail.com>
- 538f744 Merge pull request #179 from jdalton/fix/forgiving-and-eof
  Original author: Diego Perini <diego.perini@gmail.com>
- c545320 Establish shared legacy runtime initialization
  Original author: jdalton <john.david.dalton@gmail.com>
- 5d68c9b Merge pull request #197 from jdalton/refactor/legacy-runtime-init
  Original author: John-David Dalton <jdalton@users.noreply.github.com>
- ec4bcf3 Merge pull request #181 from jdalton/fix/link-precedence
  Original author: Diego Perini <diego.perini@gmail.com>
- fe15bc3 Merge pull request #183 from jdalton/perf/id-lookup
  Original author: John-David Dalton <jdalton@users.noreply.github.com>
- d7e01ee Add a jsdom selector-engine adapter
  Original author: jdalton <john.david.dalton@gmail.com>
- 62d45eb Merge pull request #211 from jdalton/feat/jsdom-adapter
  Original author: John-David Dalton <jdalton@users.noreply.github.com>
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