From 6a20b352dba085c8e9f03c0c44ab67a4dd27e244 Mon Sep 17 00:00:00 2001 From: Denys Berzoy Date: Thu, 2 Nov 2017 11:49:29 +0200 Subject: [PATCH 1/6] fix($compile): special chars are sanitized from directive name in markup this fixes bug introduced in 1.6.x when directive name with preceeding special char in HTML markup does not match the registered name --- src/ng/compile.js | 2 +- test/ng/compileSpec.js | 89 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 89 insertions(+), 2 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 561bb13fef16..15df0b5fdb8d 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -3637,7 +3637,7 @@ function SimpleChange(previous, current) { SimpleChange.prototype.isFirstChange = function() { return this.previousValue === _UNINITIALIZED_VALUE; }; -var PREFIX_REGEXP = /^((?:x|data)[:\-_])/i; +var PREFIX_REGEXP = /^((?:x|data)*[:\-_])/i; var SPECIAL_CHARS_REGEXP = /[:\-_]+(.)/g; /** diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 2890a76ac0c0..f5d6051dde16 100644 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -294,6 +294,26 @@ describe('$compile', function() { inject(function($compile) {}); }); + it('should omit special chars before processing directive name', function() { + 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('
')($rootScope); + element = $compile('
')($rootScope); + element = $compile('
')($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() { @@ -10638,7 +10658,10 @@ describe('$compile', function() { template: '
' + '
' + - '
' + '
' + + '
' + + '
' + + '
' }; }); }); @@ -10653,12 +10676,21 @@ describe('$compile', function() { var a = element.children().eq(0); var b = element.children().eq(1); var c = element.children().eq(2); + var d = element.children().eq(3); + var e = element.children().eq(4); + var f = element.children().eq(5); expect(a).toHaveClass('a'); expect(b).toHaveClass('b'); expect(c).toHaveClass('c'); + expect(d).toHaveClass('d'); + expect(e).toHaveClass('e'); + expect(f).toHaveClass('f'); expect(a.text()).toEqual('stuartbobkevin'); expect(b.text()).toEqual('stuartbobkevin'); expect(c.text()).toEqual('stuartbobkevin'); + expect(d.text()).toEqual('stuartbobkevin'); + expect(e.text()).toEqual('stuartbobkevin'); + expect(f.text()).toEqual('stuartbobkevin'); }); }); @@ -11710,6 +11742,18 @@ describe('$compile', function() { expect(element.attr('dash-test4')).toBe('JamieMason'); })); + it('should work if they are prefixed with special chars', inject(function() { + $rootScope.name = 'JamieMason'; + element = $compile('')($rootScope); + expect(element.attr('dash-test2')).toBeUndefined(); + expect(element.attr('dash-test3')).toBeUndefined(); + expect(element.attr('dash-test4')).toBeUndefined(); + $rootScope.$digest(); + expect(element.attr('dash-test2')).toBe('JamieMason'); + expect(element.attr('dash-test3')).toBe('JamieMason'); + expect(element.attr('dash-test4')).toBe('JamieMason'); + })); + it('should keep attributes ending with -start single-element directives', function() { module(function($compileProvider) { $compileProvider.directive('dashStarter', function(log) { @@ -11774,6 +11818,27 @@ describe('$compile', function() { expect(element.find('feDiffuseLighting').attr('surfaceScale')).toBe('1'); expect(element.find('feSpecularLighting').attr('surfaceScale')).toBe('1'); })); + + it('should work if they are prefixed with special chars', inject(function($compile, $rootScope) { + $rootScope.dimensions = '0 0 0 0'; + $rootScope.number = 0.42; + $rootScope.scale = 1; + + element = $compile('' + + '' + + '' + + '')($rootScope); + expect(element.attr('viewBox')).toBeUndefined(); + $rootScope.$digest(); + expect(element.attr('viewBox')).toBe('0 0 0 0'); + expect(element.find('filter').attr('filterUnits')).toBe('0.42'); + expect(element.find('feDiffuseLighting').attr('surfaceScale')).toBe('1'); + expect(element.find('feDiffuseLighting').attr('diffuseConstant')).toBe('0.42'); + expect(element.find('feSpecularLighting').attr('surfaceScale')).toBe('1'); + expect(element.find('feSpecularLighting').attr('diffuseConstant')).toBe('0.42'); + })); }); describe('multi-element directive', function() { @@ -12129,6 +12194,28 @@ describe('$compile', function() { expect(spans.eq(2)).toBeHidden(); expect(spans.eq(3)).toBeHidden(); })); + + + it('should support special char prefix', inject(function($compile, $rootScope) { + $rootScope.show = false; + element = $compile( + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '
')($rootScope); + $rootScope.$digest(); + var spans = element.find('span'); + expect(spans.eq(0)).toBeHidden(); + expect(spans.eq(1)).toBeHidden(); + expect(spans.eq(2)).toBeHidden(); + expect(spans.eq(3)).toBeHidden(); + expect(spans.eq(4)).toBeHidden(); + expect(spans.eq(5)).toBeHidden(); + })); }); describe('$animate animation hooks', function() { From 720981747a739a45fb78e5e4f8b0b12fc72e5032 Mon Sep 17 00:00:00 2001 From: Denys Berzoy Date: Mon, 6 Nov 2017 12:05:00 +0200 Subject: [PATCH 2/6] fix($compile): revert directiveNormalize behaviour to pre-1.6.x --- src/ng/compile.js | 20 +++++++++--- test/ng/compileSpec.js | 69 +----------------------------------------- 2 files changed, 17 insertions(+), 72 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 15df0b5fdb8d..38fa2d0a3312 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -3637,17 +3637,29 @@ function SimpleChange(previous, current) { SimpleChange.prototype.isFirstChange = function() { return this.previousValue === _UNINITIALIZED_VALUE; }; -var PREFIX_REGEXP = /^((?:x|data)*[:\-_])/i; -var SPECIAL_CHARS_REGEXP = /[:\-_]+(.)/g; +var PREFIX_REGEXP = /^((?:x|data)[:\-_])/i; +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) { return name - .replace(PREFIX_REGEXP, '') - .replace(SPECIAL_CHARS_REGEXP, fnCamelCaseReplace); + .replace(SPECIAL_CHARS_REGEXP, function(_, separator, letter, offset) { + return offset ? letter.toUpperCase() : letter; + }) + .replace(MOZ_HACK_REGEXP, 'Moz$1'); } /** diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index f5d6051dde16..bcc366b4c8d1 100644 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -10658,10 +10658,7 @@ describe('$compile', function() { template: '
' + '
' + - '
' + - '
' + - '
' + - '
' + '
' }; }); }); @@ -10676,21 +10673,12 @@ describe('$compile', function() { var a = element.children().eq(0); var b = element.children().eq(1); var c = element.children().eq(2); - var d = element.children().eq(3); - var e = element.children().eq(4); - var f = element.children().eq(5); expect(a).toHaveClass('a'); expect(b).toHaveClass('b'); expect(c).toHaveClass('c'); - expect(d).toHaveClass('d'); - expect(e).toHaveClass('e'); - expect(f).toHaveClass('f'); expect(a.text()).toEqual('stuartbobkevin'); expect(b.text()).toEqual('stuartbobkevin'); expect(c.text()).toEqual('stuartbobkevin'); - expect(d.text()).toEqual('stuartbobkevin'); - expect(e.text()).toEqual('stuartbobkevin'); - expect(f.text()).toEqual('stuartbobkevin'); }); }); @@ -11742,18 +11730,6 @@ describe('$compile', function() { expect(element.attr('dash-test4')).toBe('JamieMason'); })); - it('should work if they are prefixed with special chars', inject(function() { - $rootScope.name = 'JamieMason'; - element = $compile('')($rootScope); - expect(element.attr('dash-test2')).toBeUndefined(); - expect(element.attr('dash-test3')).toBeUndefined(); - expect(element.attr('dash-test4')).toBeUndefined(); - $rootScope.$digest(); - expect(element.attr('dash-test2')).toBe('JamieMason'); - expect(element.attr('dash-test3')).toBe('JamieMason'); - expect(element.attr('dash-test4')).toBe('JamieMason'); - })); - it('should keep attributes ending with -start single-element directives', function() { module(function($compileProvider) { $compileProvider.directive('dashStarter', function(log) { @@ -11818,27 +11794,6 @@ describe('$compile', function() { expect(element.find('feDiffuseLighting').attr('surfaceScale')).toBe('1'); expect(element.find('feSpecularLighting').attr('surfaceScale')).toBe('1'); })); - - it('should work if they are prefixed with special chars', inject(function($compile, $rootScope) { - $rootScope.dimensions = '0 0 0 0'; - $rootScope.number = 0.42; - $rootScope.scale = 1; - - element = $compile('' + - '' + - '' + - '')($rootScope); - expect(element.attr('viewBox')).toBeUndefined(); - $rootScope.$digest(); - expect(element.attr('viewBox')).toBe('0 0 0 0'); - expect(element.find('filter').attr('filterUnits')).toBe('0.42'); - expect(element.find('feDiffuseLighting').attr('surfaceScale')).toBe('1'); - expect(element.find('feDiffuseLighting').attr('diffuseConstant')).toBe('0.42'); - expect(element.find('feSpecularLighting').attr('surfaceScale')).toBe('1'); - expect(element.find('feSpecularLighting').attr('diffuseConstant')).toBe('0.42'); - })); }); describe('multi-element directive', function() { @@ -12194,28 +12149,6 @@ describe('$compile', function() { expect(spans.eq(2)).toBeHidden(); expect(spans.eq(3)).toBeHidden(); })); - - - it('should support special char prefix', inject(function($compile, $rootScope) { - $rootScope.show = false; - element = $compile( - '
' + - '' + - '' + - '' + - '' + - '' + - '' + - '
')($rootScope); - $rootScope.$digest(); - var spans = element.find('span'); - expect(spans.eq(0)).toBeHidden(); - expect(spans.eq(1)).toBeHidden(); - expect(spans.eq(2)).toBeHidden(); - expect(spans.eq(3)).toBeHidden(); - expect(spans.eq(4)).toBeHidden(); - expect(spans.eq(5)).toBeHidden(); - })); }); describe('$animate animation hooks', function() { From 4dc7675dc68fea2a7acfe88f7c94f6d0e739a62e Mon Sep 17 00:00:00 2001 From: Denys Berzoy Date: Mon, 6 Nov 2017 14:06:29 +0200 Subject: [PATCH 3/6] fix($compile): code review remarks fixed --- src/ng/compile.js | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 38fa2d0a3312..924815df810f 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -3639,27 +3639,17 @@ SimpleChange.prototype.isFirstChange = function() { return this.previousValue == var PREFIX_REGEXP = /^((?:x|data)[:\-_])/i; 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) { return name + .replace(PREFIX_REGEXP, '') .replace(SPECIAL_CHARS_REGEXP, function(_, separator, letter, offset) { return offset ? letter.toUpperCase() : letter; - }) - .replace(MOZ_HACK_REGEXP, 'Moz$1'); + }); } /** From e6d24392eb4a805247e6050cb0f0a2ae66b14e95 Mon Sep 17 00:00:00 2001 From: Denys Berzoy Date: Thu, 9 Nov 2017 12:07:27 +0200 Subject: [PATCH 4/6] fix($compile): regex fix --- src/ng/compile.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 924815df810f..4ec3ea5d6d94 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -3638,7 +3638,7 @@ SimpleChange.prototype.isFirstChange = function() { return this.previousValue == var PREFIX_REGEXP = /^((?:x|data)[:\-_])/i; -var SPECIAL_CHARS_REGEXP = /([:\-_]+(.))/g; +var SPECIAL_CHARS_REGEXP = /[:\-_]+(.)/g; /** * Converts all accepted directives format into proper directive name. @@ -3647,7 +3647,7 @@ var SPECIAL_CHARS_REGEXP = /([:\-_]+(.))/g; function directiveNormalize(name) { return name .replace(PREFIX_REGEXP, '') - .replace(SPECIAL_CHARS_REGEXP, function(_, separator, letter, offset) { + .replace(SPECIAL_CHARS_REGEXP, function(_, letter, offset) { return offset ? letter.toUpperCase() : letter; }); } From a3a91aa10395fa479126dfb587b9a55a6c43b874 Mon Sep 17 00:00:00 2001 From: Denys Berzoy Date: Sun, 12 Nov 2017 20:00:56 +0200 Subject: [PATCH 5/6] fix($compile): added comment to test with link to issue --- test/ng/compileSpec.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index bcc366b4c8d1..a59d119ee239 100644 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -294,7 +294,8 @@ describe('$compile', function() { inject(function($compile) {}); }); - it('should omit special chars before processing directive name', function() { + it('should omit special chars before processing attribute directive name', function() { + // a regression https://github.com/angular/angular.js/issues/16278 module(function() { directive('t', function(log) { return { From b8807dc4fb257c257089060612f9bebb9b1417f2 Mon Sep 17 00:00:00 2001 From: Denys Berzoy Date: Wed, 15 Nov 2017 10:47:16 +0200 Subject: [PATCH 6/6] fix($compile): review remarks --- test/ng/compileSpec.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index a59d119ee239..ca2d43380e8f 100644 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -294,7 +294,7 @@ describe('$compile', function() { inject(function($compile) {}); }); - it('should omit special chars before processing attribute directive name', function() { + it('should ignore special chars before processing attribute directive name', function() { // a regression https://github.com/angular/angular.js/issues/16278 module(function() { directive('t', function(log) { @@ -308,9 +308,9 @@ describe('$compile', function() { }); }); inject(function($compile, $rootScope, log) { - element = $compile('
')($rootScope); - element = $compile('
')($rootScope); - element = $compile('
')($rootScope); + $compile('
')($rootScope); + $compile('
')($rootScope); + $compile('
')($rootScope); expect(log).toEqual('pre; post; pre; post; pre; post'); }); });