Skip to content

Replace SAC selector layer with an internal Selector AST and matcher#4051

Draft
vogella wants to merge 1 commit into
eclipse-platform:masterfrom
vogella:css-replace-sac-selector-layer
Draft

Replace SAC selector layer with an internal Selector AST and matcher#4051
vogella wants to merge 1 commit into
eclipse-platform:masterfrom
vogella:css-replace-sac-selector-layer

Conversation

@vogella
Copy link
Copy Markdown
Contributor

@vogella vogella commented Jun 2, 2026

Replaces the vendored Batik/SAC selector tree with a small engine-internal selector model: a sealed Selector AST (records for type, class, id, attribute, pseudo, compound, and combinator forms) plus a static SelectorMatcher. A SAC to internal translator at the parser-output boundary feeds it, and CSSEngine.matches/parseSelectors now work on the internal AST, so 23 vendored impl/sac selector and condition wrappers fall away along with the dead ExtendedDocumentCSS query methods. Net around -1,450 lines; the bundles are internal (x-friends only) so there is no API surface change.

Specificity still follows CSS 2.1 and the static-pseudo-instance carve-out is preserved, so cascade behaviour does not shift. Sixteen new matcher unit tests cover the selector forms and specificity arithmetic, and CSSEngineTest and SelectorTest are rewritten on the internal AST. One deliberate behaviour note for reviewers: the adjacent-sibling matcher returns false (rather than throwing) on elements without sibling support. A local theme-swap benchmark on a busy workbench (20 editors, ~4000 widgets) measured a roughly 10% faster swap from removing the wrapper overhead; the larger matching and double-styling optimizations are separate later steps.

Three impl/sac classes remain as parser plumbing for a following step. Draft while the medium-risk matching path gets a closer look.

Contributes to #3980

@akurtakov
Copy link
Copy Markdown
Member

Is the faster swap coming from less SWT method calls?

@vogella
Copy link
Copy Markdown
Contributor Author

vogella commented Jun 2, 2026

Is the faster swap coming from less SWT method calls?

Less overhead in the framework , the less SWT method calls is still in the later optimization pipeline. As this is riskier I did not include that yet.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 2, 2026

Test Results

   864 files  + 3     864 suites  +3   55m 45s ⏱️ + 2m 53s
 8 049 tests +16   7 806 ✅ +17  243 💤 ±0  0 ❌  - 1 
20 586 runs  +48  19 931 ✅ +49  655 💤 ±0  0 ❌  - 1 

Results for commit 66e71d3. ± Comparison against base commit 2000403.

♻️ This comment has been updated with latest results.

Replaces the vendored Batik/SAC selector tree with a small
engine-internal selector model and a static matcher, the SAC parser
layer aside.

Adds a sealed Selector AST (records for type, class, id, attribute,
pseudo, compound, and combinator forms) plus a static SelectorMatcher
that walks an Element through it. A SAC->internal translator at the
parser-output boundary in CSSDocumentHandlerImpl.startSelector feeds
it, and CSSEngine.matches / parseSelectors now work on the internal
AST: CSSEngineImpl.matches delegates to SelectorMatcher and
applyConditionalPseudoStyle walks the internal AST instead of the SAC
tree. The parser is configured with Batik's stock
DefaultSelectorFactory and DefaultConditionFactory, so 23 vendored
impl/sac/* selector and condition wrappers fall away, along with the
dead ExtendedDocumentCSS.queryConditionSelector / querySelector methods
and the SAC_*_CONDITION constants behind them.

Specificity follows CSS 2.1 and the static-pseudo-instance carve-out
from CSSPseudoClassConditionImpl is preserved, so cascade behaviour
does not shift. Selectors.SelectorList exposes getLength()/item(int)
and every record overrides toString() to return text() for readable
selector text in error messages and rule.getSelectorText().
CSSEngineTest and SelectorTest are rewritten on the internal AST, and
sixteen new matcher unit tests cover universal, type case-sensitivity,
class, id, compound, descendant, child, attribute presence and
word-match, pseudo-class, selector lists, and specificity arithmetic.

The new package is exported x-friends to org.eclipse.e4.ui.tests.css.core
for the tests. Three impl/sac classes (CSSDocumentHandlerImpl,
DocumentHandlerFactoryImpl, SACParserFactoryImpl) remain as parser
plumbing. Net ~-1,450 LOC; bundles internal (x-friends only), no API
surface change.

Contributes to eclipse-platform#3980
@vogella vogella force-pushed the css-replace-sac-selector-layer branch from ce34aea to 66e71d3 Compare June 2, 2026 14:06
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