Apply CSS specificity when computing styles#4059
Conversation
Add @bramus/specificity to compute and compare selector specificities so stylesheet rules are applied in specificity order. Pass a specificities Map through applyStyleSheetRules and propagate selectorText from rules; refactor handleRule/handleProperty to consider inline declarations, property priorities, and selector specificity. Introduce setProperty helper to consolidate property-setting logic and simplify matches() selector checking. Includes minor formatting and argument-passing cleanup.
Handling CSS-wide keywords should be done in cssstyle.
domenic
left a comment
There was a problem hiding this comment.
Amazing work!! I'm so excited to get specificity support in jsdom after all these years.
I have one higher-level organization issue we should sort out as part of this. I'd like us to move away from the opt argument pattern. The existing code had a simple version of this, with { declaration } threaded through the call chain, but this PR significantly expands it. This makes the data flow hard to follow and fragile:
prepareComputedStyleDeclarationcreates{ declaration, specificities }and passes it toapplyStyleSheetRulesapplyStyleSheetRulespasses the sameoptthrough tohandleSheethandleSheetmutatesopt.ast = astbefore passing tohandleRulehandleRulemutatesopt.style = rule.stylebefore passing tohandlePropertyhandlePropertyfinally destructures{ ast, declaration, inline, specificities, style }: five fields, set at four different layers
I think a cleaner approach would be to use explicit parameters for each function, so each one receives only what it needs. Here's how I'd suggest restructuring the existing functions:
prepareComputedStyleDeclaration: Pass just declaration to applyStyleSheetRules (no opts bag at all). Handle inline styles separately afterward. Since inline styles always win over non-!important stylesheet declarations, they don't need specificity tracking, so you can just call declaration.setProperty directly in the loop (with appropriate !important handling) instead of routing through handleProperty with an inline: true flag.
applyStyleSheetRules: Accept (elementImpl, declaration) as direct parameters. Create specificities = new Map() internally. That's an implementation detail, not something the caller should need to know about. Pass declaration and specificities down to handleSheet.
handleSheet: Accept (sheet, elementImpl, declaration, specificities). When a rule matches, pass the ast returned by matches() directly to handleRule as a parameter, instead of stuffing it into a shared object.
handleRule: Accept (rule, ast, declaration, specificities). Pass rule.style directly to handleProperty as a parameter, instead of mutating a shared object.
handleProperty: Accept (property, style, ast, declaration, specificities). All the data it needs arrives as explicit parameters, with no reliance on a shared mutable bag.
This way each function's signature documents exactly what data it needs, there's no shared mutable object being threaded and mutated across layers, and implementation details like specificities are created where they're used rather than by a distant caller.
WDYT?
f380d29 to
d7b0673
Compare
domenic
left a comment
There was a problem hiding this comment.
Awesome, only minor suggestions remaining.
Fix #3318 and some wpt tests regarding specificity.
test/web-platform-tests/to-upstream/css/cssom/getComputedStyle-specificity.htmlwithtest/web-platform-tests/to-upstream/css/selectors/selector-specificity.html.