Skip to content

Add CSS var() function support#517

Merged
wieslawsoltes merged 10 commits into
masterfrom
issue-235-css-var-support
May 9, 2026
Merged

Add CSS var() function support#517
wieslawsoltes merged 10 commits into
masterfrom
issue-235-css-var-support

Conversation

@wieslawsoltes
Copy link
Copy Markdown
Owner

Add CSS var() Function Support

Summary

This change adds support for resolving CSS custom properties through the var() function in Svg.Custom. It addresses issue #235, where SVG content using declarations such as stroke: var(--color-text, #000) failed to render with the fallback value or with an inherited custom property value.

The implementation supports:

  • CSS custom property declarations whose names start with --.
  • var(--name) lookup through the SVG ancestor chain.
  • var(--name, fallback) fallback resolution when a custom property is missing.
  • Nested var() calls inside resolved custom property values and fallback values.
  • Cascade ordering by selector specificity and source order for custom property declarations.
  • Inline style="..." custom properties without lowercasing the custom property name.
  • Direct paint parsing fallback for unresolved variables that provide a fallback color.

Implementation Details

Custom Property Resolver

The new SvgCssVariableResolver stores custom property declarations per SvgElement using a ConditionalWeakTable. Each custom property keeps its own specificity-ordered rule set, mirroring the existing style cascade behavior well enough for the SVG load pipeline.

The resolver walks ParentsAndSelf when a consuming element references var(--name), so custom properties inherit through the SVG tree. When a custom property value itself contains var(), the nested lookup is evaluated against the element where that custom property was declared. That matches CSS custom property behavior and avoids incorrectly resolving inherited variables against the consuming child.

The resolver also avoids treating arbitrary identifiers containing var( as CSS variable functions by requiring var( to begin as a standalone function token.

Stylesheet Pipeline

SvgCssCompatibilityProcessor now applies custom property declarations before normal style application. This is needed because ExCSS does not reliably surface --* declarations through its normal declaration model. The compatibility processor scans top-level style rules from the expanded stylesheet, extracts custom property declarations, parses the selector through ExCSS, and applies matching custom properties to the same SVG elements selected by the existing selector pipeline.

Root-level selectors are handled explicitly for svg and :root, so declarations such as:

svg {
  --color-text: #00ff00;
}

are available to descendants of the root document.

Normal declarations continue to flow through AddStyle, while custom property declarations are routed into the resolver.

Inline Style and Attribute Conversion

Inline styles now route declarations through a shared helper so --* declarations are stored as custom properties instead of regular styles. Normal style properties keep their previous behavior.

SvgElementFactory.SetPropertyValue resolves var() before typed property conversion. This allows existing paint, length, opacity, and other conversion paths to receive the resolved CSS value.

SvgPaintServerFactory.Create also resolves fallback-only values such as var(--missing, #000) for direct paint conversion paths where no element context exists.

Tests

The SKSvg regression tests cover:

  • Missing custom property with fallback paint color:
    • stroke: var(--color-text, #000) renders black.
  • Inherited custom property from a stylesheet root rule:
    • svg { --color-text: #00ff00; }
    • path { stroke: var(--color-text, #000); }
    • renders green.
  • Same-specificity source order:
    • later path { --color-text: ... } declarations win over earlier declarations.
  • Nested inherited custom properties:
    • an inherited custom property containing var(--theme-color) resolves nested variables at the declaring element, not the consuming child.

Verification

The following commands passed locally:

dotnet format Svg.Skia.slnx --include src/Svg.Custom/Compatibility/SvgCssVariableResolver.cs src/Svg.Custom/Compatibility/SvgCssCompatibilityProcessor.cs src/Svg.Custom/Compatibility/SvgElementFactory.cs src/Svg.Custom/Compatibility/SvgInlineStyleAttributeParser.cs src/Svg.Custom/Painting/SvgPaintServerFactory.cs tests/Svg.Skia.UnitTests/SKSvgTests.cs --no-restore --verbosity minimal
dotnet test tests/Svg.Skia.UnitTests/Svg.Skia.UnitTests.csproj -c Release --no-restore --filter "FullyQualifiedName~Svg.Skia.UnitTests.SKSvgTests"
dotnet build Svg.Skia.slnx -c Release
dotnet test Svg.Skia.slnx -c Release

The full Release build completed with existing warnings only. The full solution test run passed, including the Svg.Skia visual/reference suite.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 30c3ae7acc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +35 to +38
while (rules.ContainsKey(specificity))
{
specificity++;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve custom-property specificity when resolving ties

Do not mutate the specificity value to break source-order ties here. Incrementing until an unused key is found can promote a later low-specificity declaration above an earlier higher-specificity one whenever intermediate keys are occupied (for example path -> svg path -> path for the same variable), so var(--x) can resolve to the wrong value. The tie-break needs a separate source-order dimension rather than changing specificity itself.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in cfd9d1f.

I replaced the specificity-key mutation with explicit cascade metadata for each custom-property rule. Each stored rule now keeps the authored value, original specificity, important priority, and a monotonic source-order value. Winner selection compares important priority first, then original specificity, then source order, so equal-specificity ties still follow source order without letting a later low-specificity rule get promoted above an earlier stronger selector.

I also added Save_CssVarPaint_DoesNotPromoteLowSpecificitySourceOrderTie, which reproduces the problematic case by filling many low-specificity path declarations around a stronger g path declaration and then adding another later path declaration. The expected result stays green from the stronger selector.

return false;
}

value = NormalizeDeclarationSegment(declarationsText, separatorIndex + 1, declarationEnd - separatorIndex - 1);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Strip !important from custom property values before storage

This parser stores the full raw value text for --* declarations, so trailing !important becomes part of the variable payload (e.g. #00ff00 !important). When var(--name) is substituted into typed properties like stroke, the converter receives an invalid token sequence and the declaration is dropped instead of honoring the variable value with priority. Custom-property parsing needs to separate priority metadata from the stored value.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in cfd9d1f.

Custom-property storage now separates a trailing !important marker from the substituted value. The resolver strips the marker before storing the value used by var(...), records priority on the custom-property rule metadata, and keeps escaped \!important text as payload rather than priority. The cascade comparison honors important declarations before specificity and source order.

I added Save_CssVarPaint_StripsImportantAndHonorsPriority, which verifies that a low-specificity important custom property overrides a higher-specificity normal custom property and substitutes the clean color token into stroke.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cfd9d1f3e8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

specificity,
important,
Interlocked.Increment(ref s_customPropertySourceOrder)));
element.CustomAttributes[name] = GetWinningRule(rules).Value;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep CSS variables out of XML custom attributes

AddCustomProperty persists winning --* values into element.CustomAttributes, but SvgElement.WriteAttributes serializes every custom attribute key as an XML attribute name. Names like --color-text are not valid XML attribute names, so loading an SVG that uses CSS custom properties and then writing/saving it can throw during serialization (or produce invalid output). These values should stay in CSS-variable resolver state (or be emitted back into style text), not in CustomAttributes.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in fb19d6485be6eb3d775e3fa8ed754428a4d57b69.

SvgCssVariableResolver.AddCustomProperty no longer writes winning --* values into SvgElement.CustomAttributes, so CSS custom properties are no longer serialized as XML attributes by SvgElement.WriteAttributes. The resolver now keeps the cascade data only in its per-element weak-table state and resolves inherited variables by walking ParentsAndSelf and reading the winning rule from that resolver state.

I also added SvgCssVariableResolver.ClearCustomProperties and call it while restoring compatibility style state before a dynamic style reapply. That prevents stale resolver entries from surviving a style mutation now that CustomAttributes is no longer the lookup source.

Coverage added: Write_CssVarPaint_DoesNotSerializeCustomPropertyAttributes loads an SVG with a stylesheet custom property, writes the SVG back out, verifies serialization does not emit --color-text="...", and verifies the stylesheet text still contains the authored custom property.

Verification:

  • dotnet test tests/Svg.Skia.UnitTests/Svg.Skia.UnitTests.csproj -f net10.0 -c Release --filter "FullyQualifiedName~SKSvgTests"
  • dotnet build Svg.Skia.slnx -c Release
  • dotnet test Svg.Skia.slnx -c Release --no-build

foreach (var declaration in declarations)
{
elem.AddStyle(declaration.Name, declaration.Value, specificity);
ApplyDeclaration(elem, declaration, specificity);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid reapplying custom properties in the second pass

Custom properties are already applied in ApplyCustomPropertyRules before the main stylesheet loop. Calling ApplyDeclaration again in the main loop re-adds any --* declarations that ExCSS happens to expose, giving them a newer SourceOrder in SvgCssVariableResolver. If ExCSS exposes only some custom-property declarations (the exact compatibility gap this patch targets), duplicated earlier rules can incorrectly outrank later rules that were only seen in the first pass.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in fb19d6485be6eb3d775e3fa8ed754428a4d57b69.

SvgCssCompatibilityProcessor.ApplyDeclaration now skips custom-property declarations in the main ExCSS stylesheet pass. The raw CSS custom-property pass (ApplyCustomPropertyRules) remains the single source of stylesheet --* cascade entries, so ExCSS-exposed custom properties cannot be re-added later with a newer resolver source order after raw-only custom-property rules have already been processed.

This keeps custom-property source ordering stable across mixed parser behavior: normal typed declarations still go through element.AddStyle, while stylesheet custom properties are applied exactly once through the raw declaration reader. Inline style="--x: ..." and XML/presentation-style custom property paths still call SvgCssVariableResolver.AddCustomProperty directly, so non-stylesheet variable support is unchanged.

The same commit also keeps the resolver state out of XML CustomAttributes and clears per-element custom-property resolver state before compatibility styles are reapplied, so dynamic style updates do not retain stale custom-property rules.

Verification:

  • dotnet test tests/Svg.Skia.UnitTests/Svg.Skia.UnitTests.csproj -f net10.0 -c Release --filter "FullyQualifiedName~SKSvgTests"
  • dotnet build Svg.Skia.slnx -c Release
  • dotnet test Svg.Skia.slnx -c Release --no-build

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f96b0c1307

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +31 to +35
var propertyRules = s_customPropertyRules.GetOrCreateValue(element);
if (!propertyRules.TryGetValue(name, out var rules))
{
rules = new List<CustomPropertyRule>();
propertyRules[name] = rules;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Clear stale custom-property rules on style reapply

When compatibility styles are reapplied after DOM/style changes, custom-property state from the previous pass is never removed: AddCustomProperty only appends to the per-element ConditionalWeakTable and updates CustomAttributes, while RestoreCompatibilityStyleState only clears known presentation attributes and leaves --* entries untouched. In a dynamic document, removing --c from a <style> block or changing it from a more-specific selector to a less-specific one leaves the old rule available to later var(--c) resolution, so repaints can keep using a value that is no longer in the stylesheet.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in fb19d6485be6eb3d775e3fa8ed754428a4d57b69.

This was addressed by adding SvgCssVariableResolver.ClearCustomProperties(SvgElement) and calling it from SvgElement.RestoreCompatibilityStyleState before known presentation attributes and inline style state are restored. Compatibility style reapply now clears the previous per-element custom-property resolver table entry before stylesheet and inline styles are applied again, so removed or weakened --* declarations cannot remain available to later var(...) resolution.

The same commit also moved custom-property lookup entirely into resolver state instead of CustomAttributes, so there is no second serialized/custom-attribute copy that could preserve stale --* values after reapply. Stylesheet custom properties are then re-added only by ApplyCustomPropertyRules, and inline custom properties are re-added by SvgInlineStyleAttributeParser during the normal ApplyInlineStyles step.

Coverage added in this commit focuses on the serialization failure mode from the same root issue: Write_CssVarPaint_DoesNotSerializeCustomPropertyAttributes verifies custom properties stay out of XML attributes while authored stylesheet text remains intact. Existing CSS variable render tests continue to cover inherited and cascade-based resolution after the storage change.

Verification:

  • dotnet test tests/Svg.Skia.UnitTests/Svg.Skia.UnitTests.csproj -f net10.0 -c Release --filter "FullyQualifiedName~SKSvgTests"
  • dotnet build Svg.Skia.slnx -c Release
  • dotnet test Svg.Skia.slnx -c Release --no-build

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fb19d6485b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +181 to +185
foreach (var elem in elemsToStyle.Distinct())
{
foreach (var declaration in declarations)
{
SvgCssVariableResolver.AddCustomProperty(elem, declaration.Name, declaration.Value, specificity);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Track custom-property-only matches for style restore

Even with ClearCustomProperties now running during restore, elements that only receive --* declarations in this raw-CSS pass are never registered via TrackCompatibilityStyleApplication. In PreserveJavaScriptDomState mode the restore path only clears stored snapshots and restore candidates, so a rule such as svg { --c: red } can leave a stale resolver entry on the root after the stylesheet is edited or removed, and descendants using var(--c) keep resolving the old value. The fresh evidence is that this custom-property pass adds resolver state here but still has no corresponding tracking call.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in fd5c458b1bc78934d138001dd104308f4ac53874.

The raw custom-property pass now tracks every element that receives --* resolver state when PreserveJavaScriptDomState is active. That means custom-property-only selector matches, including root rules that do not apply normal presentation attributes, are added to the compatibility style restore candidate list. During a later style reapply, RestoreCompatibilityStyleState reaches those elements and SvgCssVariableResolver.ClearCustomProperties removes the previous resolver table entry before the current stylesheet and inline styles are applied again.

I added Runtime_RootClassMutationClearsCustomPropertyOnlyStyleState to cover the stale-state case. The test loads :root.theme { --target-fill: red; } with rect { fill: var(--target-fill, blue); }, removes the theme class from the SVG root through the JS DOM, and verifies the rect falls back to blue instead of keeping the previous red custom-property value.

Verification:

  • dotnet test tests/Svg.Skia.UnitTests/Svg.Skia.UnitTests.csproj -f net10.0 -c Release --filter "FullyQualifiedName~SKSvgTests|FullyQualifiedName~SvgJavaScriptRuntimeTests"
  • dotnet build Svg.Skia.slnx -c Release
  • dotnet test Svg.Skia.slnx -c Release --no-build
  • git diff --check

Comment on lines +242 to +246
private static bool SelectorSegmentTargetsSvgRoot(ReadOnlySpan<char> selector)
{
var trimmed = TrimWhitespace(selector);
return trimmed.Equals("svg".AsSpan(), StringComparison.OrdinalIgnoreCase) ||
trimmed.Equals(":root".AsSpan(), StringComparison.OrdinalIgnoreCase);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Match qualified :root selectors for variables

This only special-cases selector segments that are exactly :root, but the selector adapter's synthetic wrapper means :root.foo or :root[theme] does not resolve to the SVG document element. In SVGs that scope variables with a qualified root selector, e.g. <svg class="dark"> plus :root.dark { --c: red }, the custom property is never stored and descendant var(--c) declarations fall back even though browsers match the root element.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in fd5c458b1bc78934d138001dd104308f4ac53874.

The custom-property root special case now handles qualified simple :root selectors instead of only accepting a selector segment that is exactly :root. The matcher evaluates class, id, and presence/exact attribute qualifiers directly against the real SvgDocument root, so selectors like :root.dark and :root[theme] can store variables on the SVG root when the qualifiers match. It still avoids widening complex selectors that do not directly target the root.

I added Save_CssVarPaint_UsesQualifiedRootCustomProperty, which renders an SVG with class="dark", a :root.dark { --color-text: #00ff00; } rule, and a path stroke using var(--color-text, #000). The rendered stroke is now green, which confirms the qualified root variable is available to descendants.

The JS reapply regression added in the same commit also uses :root.theme, so it covers both initial qualified-root matching and the restore-path behavior when the root qualifier stops matching.

Verification:

  • dotnet test tests/Svg.Skia.UnitTests/Svg.Skia.UnitTests.csproj -f net10.0 -c Release --filter "FullyQualifiedName~SKSvgTests|FullyQualifiedName~SvgJavaScriptRuntimeTests"
  • dotnet build Svg.Skia.slnx -c Release
  • dotnet test Svg.Skia.slnx -c Release --no-build
  • git diff --check

…to issue-235-css-var-support

# Conflicts:
#	tests/Svg.Skia.UnitTests/SKSvgTests.cs
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: df608249c5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +348 to +350
if (!TryReadDeclaration(declarationsText, ref index, out var name, out var value))
{
return result;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Continue scanning after malformed declarations

If a malformed declaration appears before a custom property in the same block (for example bad; --color-text: #0f0; or a CSS-hack declaration), TryReadDeclaration has already advanced past that bad segment, but returning here abandons the rest of the block. Browser CSS error recovery continues after the semicolon, so later valid --* declarations should still be stored; otherwise var(--color-text) incorrectly falls back or fails for stylesheets that contain one invalid declaration before the variable.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 217feac3f4c517cc5b74093bd80b4d024cb0f37f.

CreateCustomPropertyDeclarations now keeps scanning when the declaration parser rejects a malformed declaration after advancing past it. If parsing fails without advancing, it still returns immediately to avoid an infinite loop. This preserves browser-style recovery for blocks such as bad; --color-text: #00ff00;, so a bad declaration no longer prevents later custom properties from entering the resolver state.

I added Save_CssVarPaint_ContinuesAfterMalformedCustomPropertyDeclaration to cover the reported case: path { bad; --color-text: #00ff00; } followed by stroke: var(--color-text, #000) now renders the stroke green.

Verification run, without formatting:

  • dotnet test tests/Svg.Skia.UnitTests/Svg.Skia.UnitTests.csproj -f net10.0 -c Release --filter "FullyQualifiedName~SKSvgTests|FullyQualifiedName~SvgJavaScriptRuntimeTests"
  • dotnet build Svg.Skia.slnx -c Release
  • dotnet test Svg.Skia.slnx -c Release --no-build
  • git diff --check

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fd5c458b1b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +30 to +31
var normalizedValue = NormalizeCustomPropertyValue(value, out var important);
var propertyRules = s_customPropertyRules.GetOrCreateValue(element);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Interpret CSS-wide keywords before storing custom properties

When a custom property declaration uses CSS-wide keywords such as inherit, unset, or initial, the keyword controls the custom property in the cascade rather than becoming the text substituted by var(). Storing every normalized value verbatim means a rule like svg { --c: #0f0 } path { --c: inherit; stroke: var(--c, #000) } resolves the stroke to the literal inherit instead of inheriting #0f0 for --c (and initial should make var(--c, ...) use its fallback), so valid browser-rendered SVGs can render with the wrong paint or drop the declaration.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 2694296456159da5e4c8e30ff4d5b84d4fe3d5ab.

Custom-property declarations now classify exact CSS-wide keyword values after stripping a trailing !important. A winning inherit or unset declaration no longer gets stored as literal replacement text; resolution skips that element and continues up the parent chain for the inherited custom-property value. A winning initial declaration now blocks inherited values and makes the custom property unresolved for var(...), so the var() fallback is used instead of substituting the literal initial token.

I added render coverage for both sides of the behavior: Save_CssVarPaint_CssWideInheritAndUnsetUseInheritedCustomProperty verifies inherit and unset custom properties inherit the root --color-text and render green, and Save_CssVarPaint_CssWideInitialUsesFallback verifies initial blocks the root value and renders the var() fallback blue. The inherit/unset test keeps the keyword declaration and dependent stroke: var(...) in the same selector rule to cover the reported shape of stylesheet input.

Verification run, without formatting:

  • dotnet test tests/Svg.Skia.UnitTests/Svg.Skia.UnitTests.csproj -f net10.0 -c Release --filter "FullyQualifiedName~SKSvgTests|FullyQualifiedName~SvgJavaScriptRuntimeTests"
  • dotnet build Svg.Skia.slnx -c Release
  • dotnet test Svg.Skia.slnx -c Release --no-build
  • git diff --check

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 217feac3f4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +156 to +158
if (statement.Terminator != CssStatementTerminator.Block ||
GetAtRuleKind(cssText, statement) != CssAtRuleKind.None ||
!TryGetStyleRuleParts(cssText, statement, out var selectorText, out var declarationsText))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Apply custom properties inside active media rules

When a custom property is declared inside an active conditional block such as @media screen { svg { --color-text: #0f0; } }, this raw custom-property pass skips the entire top-level at-rule before looking for nested style rules. The normal stylesheet loop now ignores --* declarations as duplicates, so those variables never enter SvgCssVariableResolver and later var(--color-text, ...) consumers render with their fallback even though browsers apply the media rule.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in eebcb5933524fab581f90dc53b23647d5802b118.

The raw custom-property pass now recognizes @media blocks and descends into them only when the existing media evaluator says the condition matches the current SVG media context. Nested matching media blocks are scanned with the same custom-property rule path as top-level rules, so declarations such as @media screen { svg { --color-text: #00ff00; } } enter SvgCssVariableResolver. Inactive media blocks, for example @media print, are skipped so their custom properties do not leak into static screen rendering.

I reused the same media-query evaluation used for import filtering and renamed that helper to describe the shared use. I added Save_CssVarPaint_AppliesCustomPropertyInsideActiveMediaRule and Save_CssVarPaint_IgnoresCustomPropertyInsideInactiveMediaRule to cover both sides of the behavior.

Verification run, without formatting:

  • dotnet test tests/Svg.Skia.UnitTests/Svg.Skia.UnitTests.csproj -f net10.0 -c Release --filter "FullyQualifiedName~SKSvgTests|FullyQualifiedName~SvgJavaScriptRuntimeTests"
  • dotnet build Svg.Skia.slnx -c Release
  • dotnet test Svg.Skia.slnx -c Release --no-build
  • git diff --check

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: eebcb59335

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +60 to +65
if (TryFindTrailingImportant(value, out var importantStartIndex))
{
important = true;
var normalizedImportantValue = value.Substring(0, importantStartIndex).TrimEnd();
valueKind = GetCustomPropertyValueKind(normalizedImportantValue);
return normalizedImportantValue;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Respect quoted !important in custom properties

When a custom property value legitimately ends with a string containing !important, such as --font: "A !important"; later consumed by font-family: var(--font), this path treats the text inside the quote as a declaration priority and stores the truncated value before the !. Because TryFindTrailingImportant does not account for quoted strings, valid custom-property payloads can be corrupted before var() substitution; the priority stripping should only apply to an unquoted top-level !important token.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant