-
Notifications
You must be signed in to change notification settings - Fork 27.4k
fix($compile): sanitize special chars in directive name #16314
Changes from 2 commits
6a20b35
7209817
4dc7675
e6d2439
a3a91aa
b8807dc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3638,16 +3638,28 @@ SimpleChange.prototype.isFirstChange = function() { return this.previousValue == | |
|
||
|
||
var PREFIX_REGEXP = /^((?:x|data)[:\-_])/i; | ||
var SPECIAL_CHARS_REGEXP = /[:\-_]+(.)/g; | ||
var SPECIAL_CHARS_REGEXP = /([:\-_]+(.))/g; | ||
var MOZ_HACK_REGEXP = /^moz([A-Z])/; | ||
|
||
/** | ||
* Converts all accepted directives format into proper directive name. | ||
* @param name Name to normalize | ||
*/ | ||
function directiveNormalize(name) { | ||
return camelCase(name.replace(PREFIX_REGEXP, '')); | ||
} | ||
|
||
/** | ||
* Converts snake_case to camelCase. | ||
* Also there is special case for Moz prefix starting with upper case letter. | ||
* @param name Name to normalize | ||
*/ | ||
function camelCase(name) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this function is only used in |
||
return name | ||
.replace(PREFIX_REGEXP, '') | ||
.replace(SPECIAL_CHARS_REGEXP, fnCamelCaseReplace); | ||
.replace(SPECIAL_CHARS_REGEXP, function(_, separator, letter, offset) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adjust arguments after removing extra parenthesis from RegExp (see previous comment). |
||
return offset ? letter.toUpperCase() : letter; | ||
}) | ||
.replace(MOZ_HACK_REGEXP, 'Moz$1'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AFAICT, this was only necessary for CSS, so we don't need it here any more. |
||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -294,6 +294,26 @@ describe('$compile', function() { | |
inject(function($compile) {}); | ||
}); | ||
|
||
it('should omit special chars before processing directive name', function() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this applies only to directives with restrict: 'A', then the test description should reflect that. Maybe even link to this issue in a comment. |
||
module(function() { | ||
directive('t', function(log) { | ||
return { | ||
restrict: 'A', | ||
link: { | ||
pre: log.fn('pre'), | ||
post: log.fn('post') | ||
} | ||
}; | ||
}); | ||
}); | ||
inject(function($compile, $rootScope, log) { | ||
element = $compile('<div _t></div>')($rootScope); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
element = $compile('<div -t></div>')($rootScope); | ||
element = $compile('<div :t></div>')($rootScope); | ||
expect(log).toEqual('pre; post; pre; post; pre; post'); | ||
}); | ||
}); | ||
|
||
it('should throw an exception if the directive factory is not defined', function() { | ||
module(function() { | ||
expect(function() { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the extra parenthesis? They are not needed afaict (and slow the RegExp down - theoretically 😀)