fix(compiler): fix CSS animation rule scope (#56800)

puckowski · pkozlowski-opensource · commit 85f77b5cda26 · 2024-07-09T09:44:53.000+02:00
It is valid CSS to list keyframe names in an animation declaration only separating the names with a comma and no whitespace. This is typical of production builds. Updated a couple of regexes and added a couple of tests to account for this scenario. Fixes #53038 PR Close #56800
diff --git a/packages/compiler/src/shadow_css.ts b/packages/compiler/src/shadow_css.ts
@@ -334,8 +334,8 @@ export class ShadowCss {
    * animation declaration (with possibly multiple animation definitions)
    *
    * The regular expression can be divided in three parts
-   *  - (^|\s+)
-   *    simply captures how many (if any) leading whitespaces are present
+   *  - (^|\s+|,)
+   *    captures how many (if any) leading whitespaces are present or a comma
    *  - (?:(?:(['"])((?:\\\\|\\\2|(?!\2).)+)\2)|(-?[A-Za-z][\w\-]*))
    *    captures two different possible keyframes, ones which are quoted or ones which are valid css
    * idents (custom properties excluded)
@@ -344,7 +344,7 @@ export class ShadowCss {
    * semicolon or the end of the string
    */
   private _animationDeclarationKeyframesRe =
-    /(^|\s+)(?:(?:(['"])((?:\\\\|\\\2|(?!\2).)+)\2)|(-?[A-Za-z][\w\-]*))(?=[,\s]|$)/g;
+    /(^|\s+|,)(?:(?:(['"])((?:\\\\|\\\2|(?!\2).)+)\2)|(-?[A-Za-z][\w\-]*))(?=[,\s]|$)/g;
 
   /**
    * Scope an animation rule so that the keyframes mentioned in such rule
@@ -364,7 +364,7 @@ export class ShadowCss {
     unscopedKeyframesSet: ReadonlySet<string>,
   ): CssRule {
     let content = rule.content.replace(
-      /((?:^|\s+|;)(?:-webkit-)?animation(?:\s*):(?:\s*))([^;]+)/g,
+      /((?:^|\s+|;)(?:-webkit-)?animation\s*:\s*),*([^;]+)/g,
       (_, start, animationDeclarations) =>
         start +
         animationDeclarations.replace(
diff --git a/packages/compiler/test/shadow_css/keyframes_spec.ts b/packages/compiler/test/shadow_css/keyframes_spec.ts
@@ -122,6 +122,73 @@ describe('ShadowCss, keyframes and animations', () => {
     });
   });
 
+  it('should handle (scope or not) animation definition containing some names which do not have a preceding space', () => {
+    const COMPONENT_VARIABLE = '%COMP%';
+    const HOST_ATTR = `_nghost-${COMPONENT_VARIABLE}`;
+    const CONTENT_ATTR = `_ngcontent-${COMPONENT_VARIABLE}`;
+    const css = `.test {
+      animation:my-anim 1s,my-anim2 2s, my-anim3 3s,my-anim4 4s;
+    }
+    
+    @keyframes my-anim {
+      0% {color: red}
+      100% {color: blue}
+    }
+    
+    @keyframes my-anim2 {
+      0% {font-size: 1em}
+      100% {font-size: 1.2em}
+    }
+    `;
+    const result = shim(css, CONTENT_ATTR, HOST_ATTR);
+    const animationLineMatch = result.match(/animation:[^;]+;/);
+    let animationLine = '';
+    if (animationLineMatch) {
+      animationLine = animationLineMatch[0];
+    }
+    ['my-anim', 'my-anim2'].forEach((scoped) =>
+      expect(animationLine).toContain(`_ngcontent-%COMP%_${scoped}`),
+    );
+    ['my-anim3', 'my-anim4'].forEach((nonScoped) => {
+      expect(animationLine).toContain(nonScoped);
+      expect(animationLine).not.toContain(`_ngcontent-%COMP%_${nonScoped}`);
+    });
+  });
+
+  it('should handle (scope or not) animation definitions preceded by an erroneous comma', () => {
+    const COMPONENT_VARIABLE = '%COMP%';
+    const HOST_ATTR = `_nghost-${COMPONENT_VARIABLE}`;
+    const CONTENT_ATTR = `_ngcontent-${COMPONENT_VARIABLE}`;
+    const css = `.test {
+      animation:, my-anim 1s,my-anim2 2s, my-anim3 3s,my-anim4 4s;
+    }
+    
+    @keyframes my-anim {
+      0% {color: red}
+      100% {color: blue}
+    }
+    
+    @keyframes my-anim2 {
+      0% {font-size: 1em}
+      100% {font-size: 1.2em}
+    }
+    `;
+    const result = shim(css, CONTENT_ATTR, HOST_ATTR);
+    const animationLineMatch = result.match(/animation:[^;]+;/);
+    let animationLine = '';
+    if (animationLineMatch) {
+      animationLine = animationLineMatch[0];
+    }
+    expect(result).not.toContain('animation:,');
+    ['my-anim', 'my-anim2'].forEach((scoped) =>
+      expect(animationLine).toContain(`_ngcontent-%COMP%_${scoped}`),
+    );
+    ['my-anim3', 'my-anim4'].forEach((nonScoped) => {
+      expect(animationLine).toContain(nonScoped);
+      expect(animationLine).not.toContain(`_ngcontent-%COMP%_${nonScoped}`);
+    });
+  });
+
   it('should handle (scope or not) multiple animation definitions in a single declaration', () => {
     const css = `
         div {
