Skip to content

Commit

Permalink
fix(compile): Collecting directives from tags that will be replaced
Browse files Browse the repository at this point in the history
Fix the collecting of directives from elements that will get replaced
by another directive on an attribute. If the element has an attribute
or class directive that will replace the entire element, then do not
collect the directive on the element

Closes angular#2573

BREAKING CHANGE:
* Will break directives that made the assumption that any directive
on the replaced element would get collected
* Will break directives that expect to be collected twice when they
were replaced by another directive that is marked as replace
  • Loading branch information
lgalfaso committed May 9, 2013
1 parent 40c36ee commit ee5b7af
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 6 deletions.
22 changes: 16 additions & 6 deletions src/ng/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -510,10 +510,8 @@ function $CompileProvider($provide) {

switch(nodeType) {
case 1: /* Element */
// use the node name: <directive>
addDirective(directives,
directiveNormalize(nodeName_(node).toLowerCase()), 'E', maxPriority);

var attributesDirectives = [];
var hasReplaceAttributeDirective = false;
// iterate over the attributes
for (var attr, name, nName, ngAttrName, value, nAttrs = node.attributes,
j = 0, jj = nAttrs && nAttrs.length; j < jj; j++) {
Expand All @@ -534,7 +532,7 @@ function $CompileProvider($provide) {
attrs[nName] = true; // presence means true
}
addAttrInterpolateDirective(node, directives, value, nName);
addDirective(directives, nName, 'A', maxPriority);
addDirective(attributesDirectives, nName, 'A', maxPriority);
}
}

Expand All @@ -543,12 +541,24 @@ function $CompileProvider($provide) {
if (isString(className) && className !== '') {
while (match = CLASS_DIRECTIVE_REGEXP.exec(className)) {
nName = directiveNormalize(match[2]);
if (addDirective(directives, nName, 'C', maxPriority)) {
if (addDirective(attributesDirectives, nName, 'C', maxPriority)) {
attrs[nName] = trim(match[3]);
}
className = className.substr(match.index + match[0].length);
}
}

forEach(attributesDirectives, function(directive) {
hasReplaceAttributeDirective |= directive.replace;
});
if (!hasReplaceAttributeDirective) {
// use the node name: <directive>
addDirective(directives,
directiveNormalize(nodeName_(node).toLowerCase()), 'E', maxPriority);
}
forEach(attributesDirectives, function(directive) {
directives.push(directive);
});
break;
case 3: /* Text Node */
addTextInterpolateDirective(directives, node.nodeValue);
Expand Down
17 changes: 17 additions & 0 deletions test/ng/compileSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,10 @@ describe('$compile', function() {
expect(element).toBe(attr.$$element);
}
}));
directive('replaceDirective', valueFn({
replace: true,
template: '<input ng-maxlength="3" />'
}));
}));


Expand Down Expand Up @@ -575,6 +579,19 @@ describe('$compile', function() {
}));


it('should not collect the directive from an element if the element has a' +
' replace directive', inject(function($compile, $rootScope, $sniffer, $browser) {
var element = $compile('<form name="form"><input ng-model="name" name="alias" replace-directive /></form>')($rootScope);

element.find('input').val('aaaa');
browserTrigger(element.find('input'), ($sniffer.hasEvent('input')) ? 'input' : 'change');
expect($rootScope.name).toBe(undefined);
expect($rootScope.form.alias.$valid).toBe(false);

dealoc(element);
}));


it('should merge interpolated css class', inject(function($compile, $rootScope) {
element = $compile('<div class="one {{cls}} three" replace></div>')($rootScope);

Expand Down

0 comments on commit ee5b7af

Please sign in to comment.