applyThemeConfigSafely's cleanup always removes --interactive-focus-ring instead of restoring its true original value

[mikewheeleer]

Description

src/lib/embedThemeParser.ts's applyThemeConfigSafely captures the original accent-color value before applying overrides, but only for one of the two custom properties it actually writes:

export function applyThemeConfigSafely(config: ThemeConfig): () => void {
  const html = document.documentElement;
  const originalTheme = html.getAttribute("data-theme");
  const originalAccentColor = html.style.getPropertyValue("--color-accent-primary");

  try {
    if (config.theme) {
      html.setAttribute("data-theme", config.theme);
    }
    if (config.accentColor && isValidCssVariableValue(config.accentColor)) {
      html.style.setProperty("--color-accent-primary", config.accentColor);
      html.style.setProperty("--interactive-focus-ring", config.accentColor);
    }
  } catch (error) { ... }

  return () => {
    try {
      if (originalTheme) { html.setAttribute("data-theme", originalTheme); }
      else { html.removeAttribute("data-theme"); }

      if (originalAccentColor) {
        html.style.setProperty("--color-accent-primary", originalAccentColor);
      } else {
        html.style.removeProperty("--color-accent-primary");
        html.style.removeProperty("--interactive-focus-ring");
      }
    } catch (error) { ... }
  };
}

The function writes to two custom properties (--color-accent-primary and --interactive-focus-ring) but only captures the original value of one of them (originalAccentColor, for --color-accent-primary). On cleanup, --interactive-focus-ring is unconditionally removeProperty'd — never restored to whatever value it actually held before this function ran. If --interactive-focus-ring had a genuine pre-existing value distinct from the accent color (e.g. a separately-themed focus-ring color set elsewhere), that value is permanently lost after cleanup rather than restored, defeating the whole purpose of this being the "safe" apply/restore helper.

Requirements

• applyThemeConfigSafely must capture the original --interactive-focus-ring value before overwriting it, exactly as it already does for --color-accent-primary.
• Cleanup must restore that captured original value (or remove the property only if it was genuinely absent before), not unconditionally remove it.

Suggested execution

1. Add const originalFocusRing = html.style.getPropertyValue("--interactive-focus-ring"); alongside originalAccentColor.
2. In the cleanup function, restore --interactive-focus-ring from originalFocusRing the same way --color-accent-primary is restored from originalAccentColor (set if it had a value, removeProperty only if it was genuinely empty).
3. Add a test that pre-sets a distinct --interactive-focus-ring value on document.documentElement, calls applyThemeConfigSafely with an accentColor override, invokes the returned cleanup, and asserts the original --interactive-focus-ring value is restored rather than removed.

Acceptance criteria

• applyThemeConfigSafely captures the original --interactive-focus-ring value.
• Its cleanup function restores that original value instead of always removing the property.
• A regression test covers a pre-existing distinct focus-ring value surviving apply/cleanup.

Security notes

None; state-restoration correctness fix.

Guidelines

• Minimum 95% test coverage
• Timeframe: 96 hours

[drips-wave]

@mikewheeleer This issue could not be added to the Stellar Wave Program because the per-repo points budget would be exceeded (25000 / 25000 points already used). The label has been automatically removed.

The budget resets at the end of each Wave. You can review the remaining budget for your repos on the Orgs & Repos page.

ℹ️ Learn more about points budgets