From 724273abe60592da99463a73b6cdcddb4e005bb2 Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Wed, 8 Jun 2016 13:07:51 +0200 Subject: [PATCH 1/4] fix($compile): don't add merged attributes twice to $attrs In replace directives, attribute values from the template are added twice to the replaced element when both the replaced element and the template element have the attribute. This does not affect the DOM, as it normalizes duplicate values. The values are however duplicated in the $attrs object that is available to directive link functions. Fixes #8159 Closes #14737 --- src/ng/compile.js | 28 ++++++++++++++++------------ test/ng/compileSpec.js | 23 +++++++++++++++++++++++ 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 4baa53f7b79f..6406ff387dac 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -2761,18 +2761,22 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { // copy the new attributes on the old attrs object forEach(src, function(value, key) { - if (key === 'class') { - safeAddClass($element, value); - dst['class'] = (dst['class'] ? dst['class'] + ' ' : '') + value; - } else if (key === 'style') { - $element.attr('style', $element.attr('style') + ';' + value); - dst['style'] = (dst['style'] ? dst['style'] + ';' : '') + value; - // `dst` will never contain hasOwnProperty as DOM parser won't let it. - // You will get an "InvalidCharacterError: DOM Exception 5" error if you - // have an attribute like "has-own-property" or "data-has-own-property", etc. - } else if (key.charAt(0) !== '$' && !dst.hasOwnProperty(key)) { - dst[key] = value; - dstAttr[key] = srcAttr[key]; + // Check if we already set this attribute in the loop above + if (!dst[key]) { + + if (key === 'class') { + safeAddClass($element, value); + dst['class'] = (dst['class'] ? dst['class'] + ' ' : '') + value; + } else if (key === 'style') { + $element.attr('style', $element.attr('style') + ';' + value); + dst['style'] = (dst['style'] ? dst['style'] + ';' : '') + value; + // `dst` will never contain hasOwnProperty as DOM parser won't let it. + // You will get an "InvalidCharacterError: DOM Exception 5" error if you + // have an attribute like "has-own-property" or "data-has-own-property", etc. + } else if (key.charAt(0) !== '$' && !dst.hasOwnProperty(key)) { + dst[key] = value; + dstAttr[key] = srcAttr[key]; + } } }); } diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index c58fc1c8d2e0..2c64a8146486 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -980,6 +980,29 @@ describe('$compile', function() { })); + it('should not set merged attributes twice in $attrs', function() { + var attrs; + + module(function() { + directive('logAttrs', function() { + return { + link: function($scope, $element, $attrs) { + attrs = $attrs; + } + } + }) + }); + + inject(function($compile, $rootScope) { + element = $compile( + '
')($rootScope); + var div = element.find('div'); + expect(div.attr('class')).toBe('myLog log'); + expect(attrs.class).toBe('myLog log'); + }); + }); + + it('should prevent multiple templates per element', inject(function($compile) { try { $compile('
'); From 3dff1915714e1717af43e0dd1985651f74f71496 Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Thu, 9 Jun 2016 10:47:47 +0200 Subject: [PATCH 2/4] refactor checks --- src/ng/compile.js | 16 ++++++++-------- test/ng/compileSpec.js | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 6406ff387dac..7e106f22f97b 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -2761,19 +2761,19 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { // copy the new attributes on the old attrs object forEach(src, function(value, key) { - // Check if we already set this attribute in the loop above - if (!dst[key]) { + // Check if we already set this attribute in the loop above. + // `dst` will never contain hasOwnProperty as DOM parser won't let it. + // You will get an "InvalidCharacterError: DOM Exception 5" error if you + // have an attribute like "has-own-property" or "data-has-own-property", etc. + if (!dst.hasOwnProperty(key)) { if (key === 'class') { safeAddClass($element, value); - dst['class'] = (dst['class'] ? dst['class'] + ' ' : '') + value; + dst['class'] = value; } else if (key === 'style') { $element.attr('style', $element.attr('style') + ';' + value); - dst['style'] = (dst['style'] ? dst['style'] + ';' : '') + value; - // `dst` will never contain hasOwnProperty as DOM parser won't let it. - // You will get an "InvalidCharacterError: DOM Exception 5" error if you - // have an attribute like "has-own-property" or "data-has-own-property", etc. - } else if (key.charAt(0) !== '$' && !dst.hasOwnProperty(key)) { + dst['style'] = value; + } else if (key.charAt(0) !== '$') { dst[key] = value; dstAttr[key] = srcAttr[key]; } diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 2c64a8146486..668f6c687fa5 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -989,8 +989,8 @@ describe('$compile', function() { link: function($scope, $element, $attrs) { attrs = $attrs; } - } - }) + }; + }); }); inject(function($compile, $rootScope) { From 362b36cb6fc67930581bc0fdcce6ba2f7ba4efe9 Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Thu, 9 Jun 2016 12:36:28 +0200 Subject: [PATCH 3/4] try to simplify --- 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 7e106f22f97b..eb9273ea63cf 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -2765,18 +2765,8 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { // `dst` will never contain hasOwnProperty as DOM parser won't let it. // You will get an "InvalidCharacterError: DOM Exception 5" error if you // have an attribute like "has-own-property" or "data-has-own-property", etc. - if (!dst.hasOwnProperty(key)) { - - if (key === 'class') { - safeAddClass($element, value); - dst['class'] = value; - } else if (key === 'style') { - $element.attr('style', $element.attr('style') + ';' + value); - dst['style'] = value; - } else if (key.charAt(0) !== '$') { - dst[key] = value; - dstAttr[key] = srcAttr[key]; - } + if (!dst.hasOwnProperty(key) && key.charAt(0) !== '$') { + dst.$set(key, value, true, srcAttr[key]); } }); } From 5c1dc45c0aa402fd736d1999f4954c9e70cd2f1d Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Thu, 9 Jun 2016 15:32:08 +0200 Subject: [PATCH 4/4] better simplify replaceWith() already adds all attributes from the template, which means if no merging took place, we don't have to set the attributes again --- src/ng/compile.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index eb9273ea63cf..741eaeb90bbf 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -2766,7 +2766,11 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { // You will get an "InvalidCharacterError: DOM Exception 5" error if you // have an attribute like "has-own-property" or "data-has-own-property", etc. if (!dst.hasOwnProperty(key) && key.charAt(0) !== '$') { - dst.$set(key, value, true, srcAttr[key]); + dst[key] = value; + + if (key !== 'class' && key !== 'style') { + dstAttr[key] = srcAttr[key]; + } } }); }