Repository: darkreader/darkreader
Pull request #3056 (2020-07-09 13:48:00)

Fixed resolving circular CSS variables
- Fixed stack overflow on web.dev


Patch:
diff --git a/src/inject/dynamic-theme/css-rules.ts b/src/inject/dynamic-theme/css-rules.ts
index e4794a2c..7f2f0882 100644
--- a/src/inject/dynamic-theme/css-rules.ts
+++ b/src/inject/dynamic-theme/css-rules.ts
@@ -68,7 +68,7 @@ export function getCSSURLValue(cssURL: string) {
 
 export function getCSSBaseBath(url: string) {
     const cssURL = parseURL(url);
-    return `${cssURL.protocol}//${cssURL.host}${cssURL.pathname.replace(/\?.*$/, '').replace(/(\/)([^\/]+)$/i, '$1')}`;
+    return `${cssURL.origin}${cssURL.pathname.replace(/\?.*$/, '').replace(/(\/)([^\/]+)$/i, '$1')}`;
 }
 
 export function replaceCSSRelativeURLsWithAbsolute($css: string, cssBasePath: string) {
@@ -92,11 +92,28 @@ export function replaceCSSFontFace($css: string) {
 
 const varRegex = /var\((--[^\s,\(\)]+),?\s*([^\(\)]*(\([^\(\)]*\)[^\(\)]*)*\s*)\)/g;
 
-export function replaceCSSVariables(value: string, variables: Map<string, string>) {
+export function replaceCSSVariables(
+    value: string,
+    variables: Map<string, string>,
+    stack = new Set<string>(),
+) {
     let missing = false;
+    const unresolvable = new Set<string>();
     const result = value.replace(varRegex, (match, name, fallback) => {
+        if (stack.has(name)) {
+            logWarn(`Circular reference to variable ${name}`);
+            if (fallback) {
+                return fallback;
+            }
+            missing = true;
+            return match;
+        }
         if (variables.has(name)) {
-            return variables.get(name);
+            const value = variables.get(name);
+            if (value.match(varRegex)) {
+                unresolvable.add(name);
+            }
+            return value;
         } else if (fallback) {
             return fallback;
         } else {
@@ -109,7 +126,8 @@ export function replaceCSSVariables(value: string, variables: Map<string, string
         return result;
     }
     if (result.match(varRegex)) {
-        return replaceCSSVariables(result, variables);
+        unresolvable.forEach((v) => stack.add(v));
+        return replaceCSSVariables(result, variables, stack);
     }
     return result;
 }


Test patch:
diff --git a/tests/inject/dynamic.tests.ts b/tests/inject/dynamic.tests.ts
index 62740cd9..e855aaa5 100644
--- a/tests/inject/dynamic.tests.ts
+++ b/tests/inject/dynamic.tests.ts
@@ -1,4 +1,5 @@
 import {getAbsoluteURL} from '../../src/inject/dynamic-theme/url';
+import {replaceCSSVariables} from '../../src/inject/dynamic-theme/css-rules';
 
 test('Absolute URL', () => {
     expect(getAbsoluteURL('https://www.google.com', 'image.jpg')).toBe('https://www.google.com/image.jpg');
@@ -22,3 +23,75 @@ test('Absolute URL', () => {
     expect(getAbsoluteURL('path/index.html', 'image.jpg')).toBe(`${location.origin}/path/image.jpg`);
     expect(getAbsoluteURL('path/index.html', '/image.jpg?size=128')).toBe(`${location.origin}/image.jpg?size=128`);
 });
+
+test('Replace CSS variables', () => {
+    expect(
+        replaceCSSVariables(
+            'body { background: var(--bg); color: var(--text); }',
+            new Map(),
+        )
+    ).toBe('body { background: var(--bg); color: var(--text); }');
+
+    expect(
+        replaceCSSVariables(
+            'body { background: var(--bg); color: var(--text); } h1 { color: var(--text); }',
+            new Map([['--bg', 'white'], ['--text', 'black']]),
+        )
+    ).toBe('body { background: white; color: black; } h1 { color: black; }');
+
+    expect(
+        replaceCSSVariables(
+            'body { color: var(--text, red); }',
+            new Map([['--text', 'black']]),
+        )
+    ).toBe('body { color: black; }');
+
+    expect(
+        replaceCSSVariables(
+            'body { color: var(--text, red); }',
+            new Map(),
+        )
+    ).toBe('body { color: red; }');
+
+    expect(
+        replaceCSSVariables(
+            'body { color: var(--text, var(--alert)); }',
+            new Map([['--alert', 'red']]),
+        )
+    ).toBe('body { color: red; }');
+
+    expect(
+        replaceCSSVariables(
+            'body { color: var(--text, var(--alert, green)); }',
+            new Map(),
+        )
+    ).toBe('body { color: green; }');
+
+    expect(
+        replaceCSSVariables(
+            'body { color: var(--text); }',
+            new Map([['--text', 'var(--text)']]),
+        )
+    ).toBe('body { color: var(--text); }');
+
+    expect(
+        replaceCSSVariables(
+            'body { color: var(--text); }',
+            new Map([['--text', 'var(--alert)'], ['--alert', 'var(--text)']]),
+        )
+    ).toBe('body { color: var(--text); }');
+
+    expect(
+        replaceCSSVariables(
+            'body { color: var(--text, var(--alert)); } h1 { color: var(--text); }',
+            new Map([['--text', 'var(--alert)'], ['--alert', 'var(--text)']]),
+        )
+    ).toBe('body { color: var(--text); } h1 { color: var(--text); }');
+
+    expect(
+        replaceCSSVariables(
+            'body { color: var(--text); } h1 { color: var(--alert); }',
+            new Map([['--text', 'var(--alert)'], ['--alert', 'red']]),
+        )
+    ).toBe('body { color: red; } h1 { color: red; }');
+});
diff --git a/tests/inject/jest.config.js b/tests/inject/jest.config.js
index bb1ff3e4..3b1f2d7b 100644
--- a/tests/inject/jest.config.js
+++ b/tests/inject/jest.config.js
@@ -21,6 +21,7 @@ module.exports = {
     globals: {
         'ts-jest': {
             tsConfig: './tests/inject/tsconfig.json'
-        }
+        },
+        __DEBUG__: false,
     }
 };