Skip to content

Commit 387e1cb

Browse files
puckowskipkozlowski-opensource
authored andcommitted
fix(compiler): fix CSS animation rule scope (angular#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 angular#53038 PR Close angular#56800
1 parent 02a31c5 commit 387e1cb

File tree

2 files changed

+71
-4
lines changed

2 files changed

+71
-4
lines changed

packages/compiler/src/shadow_css.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -334,8 +334,8 @@ export class ShadowCss {
334334
* animation declaration (with possibly multiple animation definitions)
335335
*
336336
* The regular expression can be divided in three parts
337-
* - (^|\s+)
338-
* simply captures how many (if any) leading whitespaces are present
337+
* - (^|\s+|,)
338+
* captures how many (if any) leading whitespaces are present or a comma
339339
* - (?:(?:(['"])((?:\\\\|\\\2|(?!\2).)+)\2)|(-?[A-Za-z][\w\-]*))
340340
* captures two different possible keyframes, ones which are quoted or ones which are valid css
341341
* idents (custom properties excluded)
@@ -344,7 +344,7 @@ export class ShadowCss {
344344
* semicolon or the end of the string
345345
*/
346346
private _animationDeclarationKeyframesRe =
347-
/(^|\s+)(?:(?:(['"])((?:\\\\|\\\2|(?!\2).)+)\2)|(-?[A-Za-z][\w\-]*))(?=[,\s]|$)/g;
347+
/(^|\s+|,)(?:(?:(['"])((?:\\\\|\\\2|(?!\2).)+)\2)|(-?[A-Za-z][\w\-]*))(?=[,\s]|$)/g;
348348

349349
/**
350350
* Scope an animation rule so that the keyframes mentioned in such rule
@@ -364,7 +364,7 @@ export class ShadowCss {
364364
unscopedKeyframesSet: ReadonlySet<string>,
365365
): CssRule {
366366
let content = rule.content.replace(
367-
/((?:^|\s+|;)(?:-webkit-)?animation(?:\s*):(?:\s*))([^;]+)/g,
367+
/((?:^|\s+|;)(?:-webkit-)?animation\s*:\s*),*([^;]+)/g,
368368
(_, start, animationDeclarations) =>
369369
start +
370370
animationDeclarations.replace(

packages/compiler/test/shadow_css/keyframes_spec.ts

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,73 @@ describe('ShadowCss, keyframes and animations', () => {
122122
});
123123
});
124124

125+
it('should handle (scope or not) animation definition containing some names which do not have a preceding space', () => {
126+
const COMPONENT_VARIABLE = '%COMP%';
127+
const HOST_ATTR = `_nghost-${COMPONENT_VARIABLE}`;
128+
const CONTENT_ATTR = `_ngcontent-${COMPONENT_VARIABLE}`;
129+
const css = `.test {
130+
animation:my-anim 1s,my-anim2 2s, my-anim3 3s,my-anim4 4s;
131+
}
132+
133+
@keyframes my-anim {
134+
0% {color: red}
135+
100% {color: blue}
136+
}
137+
138+
@keyframes my-anim2 {
139+
0% {font-size: 1em}
140+
100% {font-size: 1.2em}
141+
}
142+
`;
143+
const result = shim(css, CONTENT_ATTR, HOST_ATTR);
144+
const animationLineMatch = result.match(/animation:[^;]+;/);
145+
let animationLine = '';
146+
if (animationLineMatch) {
147+
animationLine = animationLineMatch[0];
148+
}
149+
['my-anim', 'my-anim2'].forEach((scoped) =>
150+
expect(animationLine).toContain(`_ngcontent-%COMP%_${scoped}`),
151+
);
152+
['my-anim3', 'my-anim4'].forEach((nonScoped) => {
153+
expect(animationLine).toContain(nonScoped);
154+
expect(animationLine).not.toContain(`_ngcontent-%COMP%_${nonScoped}`);
155+
});
156+
});
157+
158+
it('should handle (scope or not) animation definitions preceded by an erroneous comma', () => {
159+
const COMPONENT_VARIABLE = '%COMP%';
160+
const HOST_ATTR = `_nghost-${COMPONENT_VARIABLE}`;
161+
const CONTENT_ATTR = `_ngcontent-${COMPONENT_VARIABLE}`;
162+
const css = `.test {
163+
animation:, my-anim 1s,my-anim2 2s, my-anim3 3s,my-anim4 4s;
164+
}
165+
166+
@keyframes my-anim {
167+
0% {color: red}
168+
100% {color: blue}
169+
}
170+
171+
@keyframes my-anim2 {
172+
0% {font-size: 1em}
173+
100% {font-size: 1.2em}
174+
}
175+
`;
176+
const result = shim(css, CONTENT_ATTR, HOST_ATTR);
177+
const animationLineMatch = result.match(/animation:[^;]+;/);
178+
let animationLine = '';
179+
if (animationLineMatch) {
180+
animationLine = animationLineMatch[0];
181+
}
182+
expect(result).not.toContain('animation:,');
183+
['my-anim', 'my-anim2'].forEach((scoped) =>
184+
expect(animationLine).toContain(`_ngcontent-%COMP%_${scoped}`),
185+
);
186+
['my-anim3', 'my-anim4'].forEach((nonScoped) => {
187+
expect(animationLine).toContain(nonScoped);
188+
expect(animationLine).not.toContain(`_ngcontent-%COMP%_${nonScoped}`);
189+
});
190+
});
191+
125192
it('should handle (scope or not) multiple animation definitions in a single declaration', () => {
126193
const css = `
127194
div {

0 commit comments

Comments
 (0)