Skip to content

Commit

Permalink
fix(ngSrc, ngSrcset, ngHref): only interpolate if all expressions are…
Browse files Browse the repository at this point in the history
… defined

BREAKING CHANGE

If `bar` is `undefined`, before `<img src="foo/{{bar}}.jpg">` yields
`<img src="foo/.jpg">`. With this change, the binding will not set `src`.

If you want the old behavior, you can do this: `<img src="foo/{{bar || ''}}.jpg">`.

The same applies for `srcset` and `href`.

Closes angular#6984
  • Loading branch information
btford committed Apr 28, 2014
1 parent fcc3a7a commit 659cd08
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 10 deletions.
22 changes: 16 additions & 6 deletions src/ng/directive/booleanAttrs.js
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ forEach(BOOLEAN_ATTR, function(propName, attrName) {
// ng-src, ng-srcset, ng-href are interpolated
forEach(['src', 'srcset', 'href'], function(attrName) {
var normalized = directiveNormalize('ng-' + attrName);
ngAttributeAliasDirectives[normalized] = function() {
ngAttributeAliasDirectives[normalized] = ['$interpolate', function($interpolate) {
return {
priority: 99, // it needs to run after the attributes are interpolated
link: function(scope, element, attr) {
Expand All @@ -379,11 +379,21 @@ forEach(['src', 'srcset', 'href'], function(attrName) {
propName = null;
}

attr.$observe(normalized, function(value) {
if (!value)
return;
var parsed = $interpolate(element.attr(attr.$attr[normalized]));

scope.$watchGroup(parsed.expressions, function (values) {
var merged = '';
for (var i = 0, ii = values.length; i < ii; i++) {

This comment has been minimized.

Copy link
@btford

btford Apr 28, 2014

Author Owner

I'm not sure how I feel about reconstructing the interpolation here, but it seemed like the best way to avoid doing extra work.

if (isDefined(values[i])) {
merged += parsed.separators[i] + values[i];
} else {
return;
}
}

attr.$set(name, value);
merged += parsed.separators[values.length];

attr.$set(name, merged);

// on IE, if "ng:src" directive declaration is used and "src" attribute doesn't exist
// then calling element.setAttribute('src', 'foo') doesn't do anything, so we need
Expand All @@ -393,5 +403,5 @@ forEach(['src', 'srcset', 'href'], function(attrName) {
});
}
};
};
}];
});
6 changes: 3 additions & 3 deletions test/ng/directive/booleanAttrsSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ describe('ngSrcset', function() {
var element = $compile('<div ng-srcset="some/{{id}} 2x"></div>')($rootScope);

$rootScope.$digest();
expect(element.attr('srcset')).toEqual('some/ 2x');
expect(element.attr('srcset')).toEqual();

$rootScope.$apply(function() {
$rootScope.id = 1;
Expand All @@ -227,7 +227,7 @@ describe('ngHref', function() {
it('should interpolate the expression and bind to href', inject(function($compile, $rootScope) {
element = $compile('<div ng-href="some/{{id}}"></div>')($rootScope);
$rootScope.$digest();
expect(element.attr('href')).toEqual('some/');
expect(element.attr('href')).toEqual();

$rootScope.$apply(function() {
$rootScope.id = 1;
Expand Down Expand Up @@ -258,7 +258,7 @@ describe('ngHref', function() {
element = $compile('<svg><a ng-href="some/{{id}}"></a></svg>')($rootScope);
var child = element.children('a');
$rootScope.$digest();
expect(child.attr('xlink:href')).toEqual('some/');
expect(child.attr('xlink:href')).toEqual();

$rootScope.$apply(function() {
$rootScope.id = 1;
Expand Down
2 changes: 1 addition & 1 deletion test/ng/directive/ngSrcsetSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ describe('ngSrcset', function() {
$rootScope.image = {};
element = $compile('<img ng-srcset="{{image.url}} 2x">')($rootScope);
$rootScope.$digest();
expect(element.attr('srcset')).toEqual(' 2x');
expect(element.attr('srcset')).toEqual();
}));
});

0 comments on commit 659cd08

Please sign in to comment.