Stop state pseudo-classes from re-entering the engine - #178
Conversation
|
@jdalton The key part of the solution I proposed was the elimination of the need to use a call to the matches function to detect if it is a native function or not. This heads off future bugs like this. |
0c2f3b6 to
c88f2fa
Compare
|
Updated after reviewing the overhead concern and testing both PRs: capturing a known platform matcher once is simpler, but #176's CommonJS path also disables browser state unless existing callers supply its new third argument. This PR preserves that behavior and now reduces the repeated lookup/detection work. My earlier wording needed two corrections. “At most once per document” was too broad for the previous one-entry cache; switching documents discarded the result. Also, a nested call returns The revised optimizationDocument matcher records now live in a The regression test alternates two documents over 50 rounds and six state pseudo-classes: one delegating host call per document in total. The cache now uses There is still a cache comparison/guard check during matching. Real browser matchers must still be called to obtain current state, just as #176 calls its captured matcher. This change does not inspect function source or assume a captured function is native. Why capturing the factory global alone is insufficientI checked #176 at Capturing Reading from the node's document provides the matcher when the factory receives only a document and supports querying iframe documents. My earlier blanket claim that cross-realm browser matchers necessarily fail was too strong; the Chromium checks here also pass for those elements. Regression coverage included in the PR
The tests and reproduction instructions are committed with the optimization. Two unrelated bugs the review turned up, now their own PRsBoth are already on master and neither is caused by this patch. #195 is #196 is The second one touches this patch indirectly. The |
c88f2fa to
45c07d5
Compare
|
@jdalton Your code makes calls repeatedly on every use to a function to detect if it is native or not native. This wastes cpu cycles and is prone to create future bugs. |
|
@charlesverge I followed up on the overhead concern with The repeated This also corrects my earlier “at most once per document” claim. At There is still a cache comparison and re-entry guard during matching. A functioning browser matcher continues to be called to obtain current selector state; #176 also calls its captured matcher for those state queries. Capturing a function alone does not establish native provenance. For the compatibility check, I tested #176 at
In #176, only the browser bootstrap supplies In a separate jsdom browser-bootstrap test where The committed coverage now checks all of these cases:
The optimization reduces repeated lookup and delegation detection while preserving changing browser state. I also corrected the earlier comment so the first explanation readers encounter reflects the current implementation. |
0b09e98 to
e6ad525
Compare
7d95120 to
eb63768
Compare
eb63768 to
7a8a32e
Compare
Stop state selectors from re-entering nwsapi through a host's
Element.matches(). Fixes #172, #171 and #177.Builds on merged #197. The matcher cache uses its shared allocator after legacy mode is selected. Modern and legacy hosts with
WeakMapretain delegation results across documents; legacy hosts without it use a bounded single-document cache. Prefixed matcher aliases are checked only in legacy mode and cached per document. Modern mode uses.matchesand retains the factory guards for CommonJS callers.Validation: all 27 Node tests pass, including jsdom 26.1.0, both legacy paths, and alias lookup counts. All 5 Chromium scenarios pass across factory shapes, iframe documents, and installation before or after caching. Reproduction commands are in
test/display-state-testing.md.