This repository was archived by the owner on Apr 12, 2024. It is now read-only.
fix($animate): improve detection and handling of invalid classNameFilter RegExp
#14913
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
test/ngAnimate/animateSpec.js
Outdated
| var errorMessage = '$animateProvider.classNameFilter(regex) prohibits accepting a regex ' + | ||
| 'value which matches/contains the "ng-animate" CSS class.'; | ||
|
|
||
| assertError(/ng-animate/, true); |
Contributor
There was a problem hiding this comment.
Looks like a good place for a custom matcher
Member
Author
There was a problem hiding this comment.
This was only used in this one test, so not worth it. (I refactored the test, so now assertError is gone anyway 😃)
gkalpak
force-pushed
the
fix-animate-invalid-classNameFilter
branch
from
b2f79fe to
6606b4f
Compare
Member
Author
|
Rebased on master and addressed comments. |
gkalpak
force-pushed
the
fix-animate-invalid-classNameFilter
branch
from
6606b4f to
899c52e
Compare
Contributor
|
There's a style error, otherwise LGTM |
…owed RegExp is used
Member
Author
|
PTAL |
Contributor
|
LGTM |
ellimist
pushed a commit
to ellimist/angular.js
that referenced
this pull request
Mar 15, 2017
…gExp is used Closes angular#14913
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Bug fix.
What is the current behavior? (You can also link to an open issue here)
$animateProviderfails to detect several invalidclassNameFilterRegExps.classNameFilterRegExp is detected as invalid,$animateProviderwill still use it (despite printing an error message saying that it is prohibited).What is the new behavior (if this is a feature change)?
$animateProviderdoes a better job detecting invalidclassNameFilterRegExps (it is still not perfect though).classNameFilterRegExp is detected as invalid,classNameFilterwill be reset tonull(to avoid using an invalid RegExp).Does this PR introduce a breaking change?
No (unless someone was hacking around the previous implementation in order to allow invalid RegExps - which they shouldn't do anyway).
Please check if the PR fulfills these requirements
Docs have been added / updated (for bug fixes / features)Other information:
This PR contains two separate commits - which fix two different things - so that they can be liked/disliked independently.