From aef166f763f6601fa91b655ebff4b82b5957d6cc Mon Sep 17 00:00:00 2001 From: Daniel Puckowski Date: Mon, 1 Jul 2024 19:22:42 -0400 Subject: [PATCH] fix(compiler): fix CSS animation rule scope (#56800) 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 --- packages/compiler/src/shadow_css.ts | 8 +-- .../test/shadow_css/keyframes_spec.ts | 67 +++++++++++++++++++ 2 files changed, 71 insertions(+), 4 deletions(-) diff --git a/packages/compiler/src/shadow_css.ts b/packages/compiler/src/shadow_css.ts index b01c534d225..66bdbb7d604 100644 --- 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, ): 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 index 53216deab65..97344480eea 100644 --- 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 {