From 840b06695f8a7e2f130aa0e9d10aceb98102d9df Mon Sep 17 00:00:00 2001
From: Marco Beretta <81851188+berry-13@users.noreply.github.com>
Date: Tue, 23 Jun 2026 16:00:58 +0200
Subject: [PATCH] fix: ignore display:none shapes and stop counting CSS color
 as paint

---
 client/src/utils/__tests__/svg.test.ts | 24 +++++++++
 client/src/utils/svg.ts                | 71 ++++++++++++++++++--------
 2 files changed, 75 insertions(+), 20 deletions(-)

diff --git a/client/src/utils/__tests__/svg.test.ts b/client/src/utils/__tests__/svg.test.ts
index b65f347cfd..0eb83997bc 100644
--- a/client/src/utils/__tests__/svg.test.ts
+++ b/client/src/utils/__tests__/svg.test.ts
@@ -210,6 +210,18 @@ describe('isMonochromeSvg', () => {
         '<svg viewBox="0 0 24 24"><style>.a{fill:currentColor}</style><path class="a" d="M4 4h16v16H4z" /></svg>';
       expect(isMonochromeSvg(svg)).toBe(true);
     });
+
+    it('does not count an unused CSS color declaration as a tone', () => {
+      const svg =
+        '<svg viewBox="0 0 24 24"><style>svg{color:#333}</style><path d="M4 4h16v16H4z" /></svg>';
+      expect(isMonochromeSvg(svg)).toBe(true);
+    });
+
+    it('resolves CSS currentColor against a CSS color declaration', () => {
+      const svg =
+        '<svg viewBox="0 0 24 24"><style>.a{color:#e00;fill:currentColor}</style><path class="a" d="M4 4h16v16H4z" /></svg>';
+      expect(isMonochromeSvg(svg)).toBe(false);
+    });
   });
 
   describe('default fills alongside <style> rules', () => {
@@ -326,6 +338,18 @@ describe('isMonochromeSvg', () => {
       expect(isMonochromeSvg(svg)).toBe(true);
     });
 
+    it('tints a glyph over a rect hidden with display="none"', () => {
+      const svg =
+        '<svg viewBox="0 0 24 24"><rect width="24" height="24" fill="#fff" display="none" /><path fill="#000" d="M4 4h16v16H4z" /></svg>';
+      expect(isMonochromeSvg(svg)).toBe(true);
+    });
+
+    it('tints a glyph over a rect hidden by a CSS display:none rule', () => {
+      const svg =
+        '<svg viewBox="0 0 24 24"><style>.bg{display:none}</style><rect class="bg" width="24" height="24" fill="#fff" /><path fill="#000" d="M4 4h16v16H4z" /></svg>';
+      expect(isMonochromeSvg(svg)).toBe(true);
+    });
+
     it('rejects a full-size percentage background rect', () => {
       const svg =
         '<svg viewBox="0 0 24 24"><rect x="0" y="0" width="100%" height="100%" fill="#eee" /><path fill="#222" /></svg>';
diff --git a/client/src/utils/svg.ts b/client/src/utils/svg.ts
index a1ed1edbcb..e8e658da3f 100644
--- a/client/src/utils/svg.ts
+++ b/client/src/utils/svg.ts
@@ -66,6 +66,8 @@ const PAINT_OPACITY = new Map([
 /** Declarations resolved from `<style>` rules for tint detection. */
 const RESOLVED_DECLS = new Set([
   'fill',
+  'color',
+  'display',
   'opacity',
   'fill-opacity',
   'stroke-opacity',
@@ -75,8 +77,9 @@ const RESOLVED_DECLS = new Set([
 /** A `<style>` rule reduced to the paint/opacity declarations we resolve. */
 type StyleRule = { selector: string; declarations: Map<string, string> };
 
-/** Matches color declarations inside a `<style>` block. */
-const CSS_COLOR_REGEX = /(?:fill|stroke|stop-color|color)\s*:\s*([^;}]+)/gi;
+/** Matches paint declarations inside a `<style>` block (not `color`, which only
+ * resolves `currentColor` rather than painting on its own). */
+const CSS_COLOR_REGEX = /(?:fill|stroke|stop-color)\s*:\s*([^;}]+)/gi;
 
 function hexToRgb(hex: string): [number, number, number] | null {
   let value = hex.slice(1);
@@ -241,6 +244,9 @@ function spansCanvas(value: string | null, canvas: number | null): boolean {
 }
 
 function isOpaqueRect(rect: Element, root: Element, rules: StyleRule[]): boolean {
+  if (isHidden(rect, root, rules)) {
+    return false;
+  }
   const fill = resolveFill(rect, root, rules) ?? '';
   if (fill === 'none' || fill === 'transparent' || paintAlpha(fill) === 0) {
     return false;
@@ -277,7 +283,7 @@ function hasOpaqueBackground(root: Element, rules: StyleRule[]): boolean {
  * the rule matches (against an inherited fixed `color`), so a fixed color is
  * preserved rather than recorded as the theme-following sentinel.
  */
-function resolveCssCurrentColor(root: Element, selector: string): string[] {
+function resolveCssCurrentColor(root: Element, selector: string, rules: StyleRule[]): string[] {
   if (selector === '') {
     return [];
   }
@@ -287,10 +293,10 @@ function resolveCssCurrentColor(root: Element, selector: string): string[] {
   } catch {
     return [CURRENT_COLOR];
   }
-  return matched.map((el) => resolveCurrentColor(el, root));
+  return matched.map((el) => resolveCurrentColor(el, root, rules));
 }
 
-function colorsFromStyleBlocks(root: Element): string[] {
+function colorsFromStyleBlocks(root: Element, rules: StyleRule[]): string[] {
   const colors: string[] = [];
   for (const style of Array.from(root.querySelectorAll('style'))) {
     for (const rule of (style.textContent ?? '').split('}')) {
@@ -302,7 +308,7 @@ function colorsFromStyleBlocks(root: Element): string[] {
       for (const match of rule.slice(brace + 1).matchAll(CSS_COLOR_REGEX)) {
         const value = match[1].trim();
         if (value.toLowerCase() === CURRENT_COLOR) {
-          colors.push(...resolveCssCurrentColor(root, selector));
+          colors.push(...resolveCssCurrentColor(root, selector, rules));
         } else {
           colors.push(value);
         }
@@ -314,18 +320,16 @@ function colorsFromStyleBlocks(root: Element): string[] {
 
 /**
  * Resolves a `currentColor` paint to the fixed `color` set on the element or an
- * ancestor, since that is what the icon actually renders. Returns `currentColor`
- * unchanged when no fixed color is in scope, meaning the paint follows the theme.
+ * ancestor (inline style, attribute, or CSS), since that is what the icon actually
+ * renders. Returns `currentColor` unchanged when no fixed color is in scope,
+ * meaning the paint follows the theme.
  */
-function resolveCurrentColor(el: Element, root: Element): string {
+function resolveCurrentColor(el: Element, root: Element, rules: StyleRule[]): string {
   let current: Element | null = el;
   while (current != null) {
-    const color = readPaint(current, 'color');
-    if (color != null) {
-      const normalized = color.trim().toLowerCase();
-      if (normalized !== '' && normalized !== 'inherit' && normalized !== CURRENT_COLOR) {
-        return normalized;
-      }
+    const color = styleValue(current, rules, 'color');
+    if (color != null && color !== '' && color !== 'inherit' && color !== CURRENT_COLOR) {
+      return color;
     }
     if (current === root) {
       break;
@@ -338,7 +342,11 @@ function resolveCurrentColor(el: Element, root: Element): string {
 function collectColors(root: Element, rules: StyleRule[]): string[] {
   const colors: string[] = [];
   for (const el of [root, ...Array.from(root.querySelectorAll('*'))]) {
-    if (el.nodeName.toLowerCase() === 'style' || isInside(el, root, FUNCTIONAL_CONTAINERS)) {
+    if (
+      el.nodeName.toLowerCase() === 'style' ||
+      isInside(el, root, FUNCTIONAL_CONTAINERS) ||
+      isHidden(el, root, rules)
+    ) {
       continue;
     }
     for (const prop of PAINT_PROPS) {
@@ -347,11 +355,11 @@ function collectColors(root: Element, rules: StyleRule[]): string[] {
         continue;
       }
       colors.push(
-        value.trim().toLowerCase() === CURRENT_COLOR ? resolveCurrentColor(el, root) : value,
+        value.trim().toLowerCase() === CURRENT_COLOR ? resolveCurrentColor(el, root, rules) : value,
       );
     }
   }
-  colors.push(...colorsFromStyleBlocks(root));
+  colors.push(...colorsFromStyleBlocks(root, rules));
   return colors
     .map((color) => color.trim().toLowerCase())
     .filter((color) => color.length > 0 && !IGNORABLE_COLORS.has(color) && paintAlpha(color) !== 0);
@@ -432,6 +440,21 @@ function styleNumber(el: Element, rules: StyleRule[], property: string): number
   return Number.isNaN(number) ? null : number;
 }
 
+/** True when the element or an ancestor is removed from rendering via `display:none`. */
+function isHidden(el: Element, root: Element, rules: StyleRule[]): boolean {
+  let current: Element | null = el;
+  while (current != null) {
+    if (styleValue(current, rules, 'display') === 'none') {
+      return true;
+    }
+    if (current === root) {
+      break;
+    }
+    current = current.parentElement;
+  }
+  return false;
+}
+
 /**
  * True when a paint is fully transparent through opacity: the element (or an
  * ancestor group) has `opacity:0`, or the inherited paint-specific opacity (e.g.
@@ -531,7 +554,11 @@ function rendersFillArea(el: Element): boolean {
  */
 function hasDefaultBlackShape(root: Element, rules: StyleRule[]): boolean {
   for (const el of Array.from(root.querySelectorAll(FILLABLE_SHAPES))) {
-    if (fillIsResolved(el, root, rules) || isInside(el, root, DEFERRED_CONTAINERS)) {
+    if (
+      fillIsResolved(el, root, rules) ||
+      isInside(el, root, DEFERRED_CONTAINERS) ||
+      isHidden(el, root, rules)
+    ) {
       continue;
     }
     if (rendersFillArea(el)) {
@@ -566,7 +593,11 @@ function targetHasDefaultBlackShape(target: Element, rules: StyleRule[]): boolea
     shapes.unshift(target);
   }
   for (const el of shapes) {
-    if (resolveFill(el, target, rules) != null || isInside(el, target, FUNCTIONAL_CONTAINERS)) {
+    if (
+      resolveFill(el, target, rules) != null ||
+      isInside(el, target, FUNCTIONAL_CONTAINERS) ||
+      isHidden(el, target, rules)
+    ) {
       continue;
     }
     if (rendersFillArea(el)) {