Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port keyframe name fix from ShadyCSS #5038

Merged
merged 3 commits into from
Jan 24, 2018
Merged

Port keyframe name fix from ShadyCSS #5038

merged 3 commits into from
Jan 24, 2018

Conversation

dfreedm
Copy link
Member

@dfreedm dfreedm commented Jan 11, 2018

Fixes #3475

// Animation names are of the form [\w-], so ensure that the name regex does not partially apply
// to similarly named keyframe names by checking for a word boundary at the beginning and
// a non-word boundary or `-` at the end.
rule.keyframesNameRx = new RegExp('\\b' + rule.keyframesName + '(?!\\B|-)', 'g');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like foo will match in a-foo

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the prefix issue, looks like we'll need something more complicated like (?:animation-name\s*:|@(?:-moz|-webkit|-ms)?-?keyframes)\s*a-foo(?!\B|-)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After reviewing this, it turns out not to matter if foo matches a-foo. We're only looking to add a suffix scoping name to the keyframe, and the regex fix makes sure that that suffix scoping is only added to animations already without a suffix.

The bug here was specifically if the match was found as a prefix for an animation, such that a would match a-foo and incorrectly add the scoping suffix as an infix modification, breaking the naming scheme.

Therefore, if a-foo is matched for foo, it will no longer match for a-foo, and all animations will have the scoping suffix added correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants