From a0552bba0c9797ed8cd9309fff18096db22eea97 Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Wed, 2 Sep 2026 12:06:44 -0700 Subject: [PATCH] Respect keyframe definition order AI assistance disclosure: Codex helped implement and verify this fix under maintainer direction. --- cli/engine/browser/injected/index.mjs | 8 +++++--- cli/engine/detect-antipatterns-browser.js | 8 +++++--- tests/detect-antipatterns-browser.test.mjs | 8 ++++++++ .../fixtures/antipatterns/linked-url-patterns.css | 14 ++++++++++++++ .../fixtures/antipatterns/linked-url-patterns.html | 1 + 5 files changed, 33 insertions(+), 6 deletions(-) diff --git a/cli/engine/browser/injected/index.mjs b/cli/engine/browser/injected/index.mjs index 6a0b024af..dd7467fbc 100644 --- a/cli/engine/browser/injected/index.mjs +++ b/cli/engine/browser/injected/index.mjs @@ -1531,7 +1531,7 @@ if (IS_BROWSER) { const parts = []; const seen = new Set(); const animationNames = new Set(); - const keyframeCandidates = []; + const keyframeCandidates = new Map(); const appendRules = (rules, requiresAppliedMatch = false) => { for (const rule of rules) { if (rule.styleSheet) { @@ -1564,7 +1564,9 @@ if (IS_BROWSER) { } const keyframesName = keyframesRuleName(rule, cssText); if (keyframesName) { - keyframeCandidates.push({ + // Keyframes do not merge: when a name is defined more than once, the + // later effective definition replaces the earlier one. + keyframeCandidates.set(keyframesName, { name: keyframesName, cssText, }); @@ -1602,7 +1604,7 @@ if (IS_BROWSER) { // animation renders. The live reference is the useful gate: it preserves // linked marquee/pulse detection without letting unreferenced keyframe // bodies feed page-level checks. - for (const candidate of keyframeCandidates) { + for (const candidate of keyframeCandidates.values()) { if (!animationNames.has(candidate.name)) continue; parts.push(candidate.cssText); } diff --git a/cli/engine/detect-antipatterns-browser.js b/cli/engine/detect-antipatterns-browser.js index 39eed8ff2..398f75ef3 100644 --- a/cli/engine/detect-antipatterns-browser.js +++ b/cli/engine/detect-antipatterns-browser.js @@ -8435,7 +8435,7 @@ if (IS_BROWSER) { const parts = []; const seen = new Set(); const animationNames = new Set(); - const keyframeCandidates = []; + const keyframeCandidates = new Map(); const appendRules = (rules, requiresAppliedMatch = false) => { for (const rule of rules) { if (rule.styleSheet) { @@ -8468,7 +8468,9 @@ if (IS_BROWSER) { } const keyframesName = keyframesRuleName(rule, cssText); if (keyframesName) { - keyframeCandidates.push({ + // Keyframes do not merge: when a name is defined more than once, the + // later effective definition replaces the earlier one. + keyframeCandidates.set(keyframesName, { name: keyframesName, cssText, }); @@ -8506,7 +8508,7 @@ if (IS_BROWSER) { // animation renders. The live reference is the useful gate: it preserves // linked marquee/pulse detection without letting unreferenced keyframe // bodies feed page-level checks. - for (const candidate of keyframeCandidates) { + for (const candidate of keyframeCandidates.values()) { if (!animationNames.has(candidate.name)) continue; parts.push(candidate.cssText); } diff --git a/tests/detect-antipatterns-browser.test.mjs b/tests/detect-antipatterns-browser.test.mjs index 463300a5c..1dddc88a3 100644 --- a/tests/detect-antipatterns-browser.test.mjs +++ b/tests/detect-antipatterns-browser.test.mjs @@ -1408,9 +1408,11 @@ describe('detectUrl — browser-only fixtures', () => { const containerBackgrounds = await linkedPage.evaluate(async () => { const activeAnimation = document.querySelector('.active-container-animation-reference'); const inactiveAnimation = document.querySelector('.inactive-container-animation-reference'); + const overriddenAnimation = document.querySelector('.overridden-keyframes-animation'); const before = { active: getComputedStyle(activeAnimation).transform, inactive: getComputedStyle(inactiveAnimation).transform, + overridden: getComputedStyle(overriddenAnimation).transform, }; await new Promise(resolve => setTimeout(resolve, 120)); return { @@ -1419,6 +1421,7 @@ describe('detectUrl — browser-only fixtures', () => { pseudoClass: getComputedStyle(document.querySelector('.inactive-pseudo-stripes')).backgroundImage, activeTransforms: [before.active, getComputedStyle(activeAnimation).transform], inactiveTransforms: [before.inactive, getComputedStyle(inactiveAnimation).transform], + overriddenTransforms: [before.overridden, getComputedStyle(overriddenAnimation).transform], }; }); assert.equal(containerBackgrounds.inactive, 'none'); @@ -1430,6 +1433,11 @@ describe('detectUrl — browser-only fixtures', () => { containerBackgrounds.inactiveTransforms[1], JSON.stringify(containerBackgrounds), ); + assert.equal( + containerBackgrounds.overriddenTransforms[0], + containerBackgrounds.overriddenTransforms[1], + JSON.stringify(containerBackgrounds), + ); const grids = linkedFindings.filter(finding => finding.type === 'codex-grid-background'); assert.equal(grids.length, 1, JSON.stringify({ linkedFindings, linkedCssom })); assert.equal(grids[0].severity, 'advisory'); diff --git a/tests/fixtures/antipatterns/linked-url-patterns.css b/tests/fixtures/antipatterns/linked-url-patterns.css index fb895350a..75c66058d 100644 --- a/tests/fixtures/antipatterns/linked-url-patterns.css +++ b/tests/fixtures/antipatterns/linked-url-patterns.css @@ -47,6 +47,20 @@ main > ::before { to { transform: translateX(-50%); } } +.overridden-keyframes-animation { + animation: overridden-horizontal-loop 2s linear infinite; +} + +@keyframes overridden-horizontal-loop { + from { transform: translateX(0); } + to { transform: translateX(-50%); } +} + +/* The later same-name definition is the one Chromium renders. */ +@keyframes overridden-horizontal-loop { + 50% { opacity: 0.4; } +} + .linked-pulse-dot { width: 8px; height: 8px; diff --git a/tests/fixtures/antipatterns/linked-url-patterns.html b/tests/fixtures/antipatterns/linked-url-patterns.html index fb2bb056c..37ed7c79a 100644 --- a/tests/fixtures/antipatterns/linked-url-patterns.html +++ b/tests/fixtures/antipatterns/linked-url-patterns.html @@ -12,6 +12,7 @@
Empty attribute-value decoy
Escaped identifier selector
Rendered linked marquee animation
+
Overridden linked keyframes
Inactive media stripes
Inactive supports stripes