From 3105ff1e2dd0f201c3f545921fc106685dac3103 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Mon, 16 Nov 2015 22:29:46 +0000 Subject: [PATCH 1/9] fix($compile): support merging special attribute names in `replace` directives When compiling a `replace` directive, the compiler merges the attributes from the replaced element onto the template element. Unfortunately, `setAttribute` and other related DOM methods do not allow certain attribute names - in particular Angular 2 style names such as `(click)` and `[value]`. This is relevant when using ngForward with Angular Material, since the `mgButton` directive uses `replace` and in the former you often use `(click)`. This fixes the problem but for those special attributes the speed is considerably slow. Closes #13317 --- src/ng/compile.js | 14 +++++++++++++- test/ng/compileSpec.js | 10 ++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index b333b079aa82..c5d1b8bbac2f 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -989,6 +989,9 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { function($injector, $interpolate, $exceptionHandler, $templateRequest, $parse, $controller, $rootScope, $document, $sce, $animate, $$sanitizeUri) { + var SIMPLE_ATTR_NAME = /^\w/; + var QUOTE_REGEX = /'/g; + var specialAttrHolder = document.createElement('div'); var Attributes = function(element, attributesToCopy) { if (attributesToCopy) { var keys = Object.keys(attributesToCopy); @@ -1168,7 +1171,11 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { if (value === null || isUndefined(value)) { this.$$element.removeAttr(attrName); } else { - this.$$element.attr(attrName, value); + if (SIMPLE_ATTR_NAME.test(attrName)) { + this.$$element.attr(attrName, value); + } else { + setSpecialAttr(this.$$element[0], attrName, value); + } } } @@ -1221,6 +1228,11 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } }; + function setSpecialAttr(element, attrName, value) { + specialAttrHolder.innerHTML = ""; + var attribute = specialAttrHolder.firstChild.attributes[0].cloneNode(); + element.attributes.setNamedItem(attribute); + } function safeAddClass($element, className) { try { diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index ee09676bc4dd..db4babea4abe 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -868,6 +868,16 @@ describe('$compile', function() { expect(div.attr('id')).toEqual('myid'); })); + + it('should correctly merge attributes that contain special characters', inject(function($compile, $rootScope) { + element = $compile( + '
')($rootScope); + var div = element.find('div'); + expect(div.attr('(click)')).toEqual('doSomething()'); + expect(div.attr('[value]')).toEqual('someExpression'); + })); + + it('should prevent multiple templates per element', inject(function($compile) { try { $compile('
'); From a43867c3090eafd9ef2f62fd2259e06572c64154 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Tue, 17 Nov 2015 14:42:34 +0000 Subject: [PATCH 2/9] fix($compile): support merging special attribute names in `replace` directives When compiling a `replace` directive, the compiler merges the attributes from the replaced element onto the template element. Unfortunately, `setAttribute` and other related DOM methods do not allow certain attribute names - in particular Angular 2 style names such as `(click)` and `[value]`. This is relevant when using ngForward with Angular Material, since the `mgButton` directive uses `replace` and in the former you often use `(click)`. This fixes the problem but for those special attributes the speed is considerably slow. Closes #13317 --- src/ng/compile.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index c5d1b8bbac2f..5a7b91eaf385 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -1229,8 +1229,11 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { }; function setSpecialAttr(element, attrName, value) { - specialAttrHolder.innerHTML = ""; - var attribute = specialAttrHolder.firstChild.attributes[0].cloneNode(); + specialAttrHolder.innerHTML = "" + var span = specialAttrHolder.firstChild; + var attribute = span.attributes[0]; + // We have to remove the attribute from the holder element before we can add it to the destination element + span.removeAttribute(attrName); element.attributes.setNamedItem(attribute); } From 0b56425df9a7fd95dc904de5e98dc4c495ee3183 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Tue, 17 Nov 2015 15:04:33 +0000 Subject: [PATCH 3/9] fix($compile): support merging special attribute names in `replace` directives When compiling a `replace` directive, the compiler merges the attributes from the replaced element onto the template element. Unfortunately, `setAttribute` and other related DOM methods do not allow certain attribute names - in particular Angular 2 style names such as `(click)` and `[value]`. This is relevant when using ngForward with Angular Material, since the `mgButton` directive uses `replace` and in the former you often use `(click)`. This fixes the problem but for those special attributes the speed is considerably slow. Closes #13317 Closes #13318 --- 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 5a7b91eaf385..86ed715436d2 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -1229,11 +1229,15 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { }; function setSpecialAttr(element, attrName, value) { - specialAttrHolder.innerHTML = "" + // Attributes names that do not start with letters cannot be set using `setAttribute` + // so we have to jump through some hoops to get such an attribute + // https://github.com/angular/angular.js/pull/13318 + specialAttrHolder.innerHTML = ""; var span = specialAttrHolder.firstChild; var attribute = span.attributes[0]; // We have to remove the attribute from the holder element before we can add it to the destination element span.removeAttribute(attrName); + attribute.value = value; element.attributes.setNamedItem(attribute); } From f84a1bd1a4423a517230d1b22ee2a538136d71e3 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Tue, 17 Nov 2015 21:18:48 +0000 Subject: [PATCH 4/9] fix($compile): support merging special attribute names in `replace` directives When compiling a `replace` directive, the compiler merges the attributes from the replaced element onto the template element. Unfortunately, `setAttribute` and other related DOM methods do not allow certain attribute names - in particular Angular 2 style names such as `(click)` and `[value]`. This is relevant when using ngForward with Angular Material, since the `mgButton` directive uses `replace` and in the former you often use `(click)`. This fixes the problem but for those special attributes the speed is considerably slow. Closes #13317 Closes #13318 --- src/ng/compile.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 86ed715436d2..2bf03f968ed6 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -990,7 +990,6 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { $controller, $rootScope, $document, $sce, $animate, $$sanitizeUri) { var SIMPLE_ATTR_NAME = /^\w/; - var QUOTE_REGEX = /'/g; var specialAttrHolder = document.createElement('div'); var Attributes = function(element, attributesToCopy) { if (attributesToCopy) { @@ -1229,7 +1228,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { }; function setSpecialAttr(element, attrName, value) { - // Attributes names that do not start with letters cannot be set using `setAttribute` + // Attributes names that do not start with letters (such as `(click)`) cannot be set using `setAttribute` // so we have to jump through some hoops to get such an attribute // https://github.com/angular/angular.js/pull/13318 specialAttrHolder.innerHTML = ""; From c93d5cf0b1c30700f1c17c8c37846cf26aae80be Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Wed, 18 Nov 2015 12:11:09 +0000 Subject: [PATCH 5/9] fix($compile): support merging special attribute names in `replace` directives When compiling a `replace` directive, the compiler merges the attributes from the replaced element onto the template element. Unfortunately, `setAttribute` and other related DOM methods do not allow certain attribute names - in particular Angular 2 style names such as `(click)` and `[value]`. This is relevant when using ngForward with Angular Material, since the `mgButton` directive uses `replace` and in the former you often use `(click)`. This fixes the problem but for those special attributes the speed is considerably slow. Closes #13317 Closes #13318 --- src/ng/compile.js | 8 ++++---- test/ng/compileSpec.js | 3 ++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 2bf03f968ed6..882df65824c5 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -1232,10 +1232,10 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { // so we have to jump through some hoops to get such an attribute // https://github.com/angular/angular.js/pull/13318 specialAttrHolder.innerHTML = ""; - var span = specialAttrHolder.firstChild; - var attribute = span.attributes[0]; - // We have to remove the attribute from the holder element before we can add it to the destination element - span.removeAttribute(attrName); + var attributes = specialAttrHolder.firstChild.attributes; + var attribute = attributes[0]; + // We have to remove the attribute from its container element before we can add it to the destination element + attributes.removeNamedItem(attribute.name); attribute.value = value; element.attributes.setNamedItem(attribute); } diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index db4babea4abe..b8041d29f259 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -871,10 +871,11 @@ describe('$compile', function() { it('should correctly merge attributes that contain special characters', inject(function($compile, $rootScope) { element = $compile( - '
')($rootScope); + '
')($rootScope); var div = element.find('div'); expect(div.attr('(click)')).toEqual('doSomething()'); expect(div.attr('[value]')).toEqual('someExpression'); + expect(div.attr('Ω')).toEqual('omega'); })); From b0fda7f0d996de5d758f4840c4855167a84d6904 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Wed, 18 Nov 2015 14:39:08 +0000 Subject: [PATCH 6/9] TESTING --- src/ng/compile.js | 13 +++++++++++++ test/ng/compileSpec.js | 6 ++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 882df65824c5..b77cf1689b13 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -1232,12 +1232,25 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { // so we have to jump through some hoops to get such an attribute // https://github.com/angular/angular.js/pull/13318 specialAttrHolder.innerHTML = ""; + console.log('holder'); + console.log(specialAttrHolder); var attributes = specialAttrHolder.firstChild.attributes; var attribute = attributes[0]; + console.log('attribute'); + console.log(attribute, attribute.name, attribute.value); // We have to remove the attribute from its container element before we can add it to the destination element attributes.removeNamedItem(attribute.name); attribute.value = value; + console.log('attribute with value'); + console.log(attribute, attribute.name, attribute.value); element.attributes.setNamedItem(attribute); + console.log('element attributes'); + for(var i=0; i< element.attributes.length; i++) { + var attribute = element.attributes[i]; + console.log(attribute, attribute.name, attribute.value); + } + console.log('element'); + console.log(element); } function safeAddClass($element, className) { diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index b8041d29f259..261b743427a1 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -869,12 +869,10 @@ describe('$compile', function() { })); - it('should correctly merge attributes that contain special characters', inject(function($compile, $rootScope) { + iit('should correctly merge attributes that contain special characters', inject(function($compile, $rootScope) { element = $compile( - '
')($rootScope); + '
')($rootScope); var div = element.find('div'); - expect(div.attr('(click)')).toEqual('doSomething()'); - expect(div.attr('[value]')).toEqual('someExpression'); expect(div.attr('Ω')).toEqual('omega'); })); From 74bf1940149af536e429883eb3020368c4156d15 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Wed, 18 Nov 2015 17:54:35 +0000 Subject: [PATCH 7/9] TESTING2 --- test/ng/compileSpec.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 261b743427a1..b634648b7920 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -871,8 +871,10 @@ describe('$compile', function() { iit('should correctly merge attributes that contain special characters', inject(function($compile, $rootScope) { element = $compile( - '
')($rootScope); + '
')($rootScope); var div = element.find('div'); + expect(div.attr('(click)')).toEqual('doSomething()'); + expect(div.attr('[value]')).toEqual('someExpression'); expect(div.attr('Ω')).toEqual('omega'); })); From ee8e22af83611328f2870e8474f0775d767a7784 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Wed, 18 Nov 2015 17:58:16 +0000 Subject: [PATCH 8/9] TESTING3 --- test/ng/compileSpec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index b634648b7920..b8041d29f259 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -869,7 +869,7 @@ describe('$compile', function() { })); - iit('should correctly merge attributes that contain special characters', inject(function($compile, $rootScope) { + it('should correctly merge attributes that contain special characters', inject(function($compile, $rootScope) { element = $compile( '
')($rootScope); var div = element.find('div'); From 37857247f99b85c97e445f14ab06f6d01169eca0 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Thu, 19 Nov 2015 08:52:04 +0000 Subject: [PATCH 9/9] fix($compile): support merging special attribute names in `replace` directives When compiling a `replace` directive, the compiler merges the attributes from the replaced element onto the template element. Unfortunately, `setAttribute` and other related DOM methods do not allow certain attribute names - in particular Angular 2 style names such as `(click)` and `[value]`. This is relevant when using ngForward with Angular Material, since the `mgButton` directive uses `replace` and in the former you often use `(click)`. This fixes the problem but for those special attributes the speed is considerably slow. Closes #13317 Closes #13318 --- src/ng/compile.js | 13 ------------- test/ng/compileSpec.js | 4 ++-- 2 files changed, 2 insertions(+), 15 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index b77cf1689b13..882df65824c5 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -1232,25 +1232,12 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { // so we have to jump through some hoops to get such an attribute // https://github.com/angular/angular.js/pull/13318 specialAttrHolder.innerHTML = ""; - console.log('holder'); - console.log(specialAttrHolder); var attributes = specialAttrHolder.firstChild.attributes; var attribute = attributes[0]; - console.log('attribute'); - console.log(attribute, attribute.name, attribute.value); // We have to remove the attribute from its container element before we can add it to the destination element attributes.removeNamedItem(attribute.name); attribute.value = value; - console.log('attribute with value'); - console.log(attribute, attribute.name, attribute.value); element.attributes.setNamedItem(attribute); - console.log('element attributes'); - for(var i=0; i< element.attributes.length; i++) { - var attribute = element.attributes[i]; - console.log(attribute, attribute.name, attribute.value); - } - console.log('element'); - console.log(element); } function safeAddClass($element, className) { diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index b8041d29f259..069bdec48639 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -871,11 +871,11 @@ describe('$compile', function() { it('should correctly merge attributes that contain special characters', inject(function($compile, $rootScope) { element = $compile( - '
')($rootScope); + '
')($rootScope); var div = element.find('div'); expect(div.attr('(click)')).toEqual('doSomething()'); expect(div.attr('[value]')).toEqual('someExpression'); - expect(div.attr('Ω')).toEqual('omega'); + expect(div.attr('ω')).toEqual('omega'); }));