New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ngHref prematurely adds href attribute when string is included as part of the expression #6984

Closed
briangershon opened this Issue Apr 3, 2014 · 5 comments

Comments

Projects
None yet
2 participants
@briangershon

The ngHref doc page gives an example of the correct way of using ngHref to avoid broken URLs until {{hash}} is evaluated.

<a ng-href="http://www.gravatar.com/avatar/{{hash}}"/>

I've created a plunker that demonstrates that this generates the href attribute right away, prior to the {{hash}} evaluating, so the full url is not yet valid.

My understanding is that we should not see either link until {{hash}} is filled in (via the input box), but in the first link example in this plunker we do see an active link:

http://plnkr.co/EOWiCm

I traced through and it seems that the $observe() call is placed on the whole ngHref value and not just the {{hash}}.

Seeing this happen in Angular 1.2.9 through 1.3.0-beta.4 (which the Plunker example is using). Reproduced in Chrome and Safari.

I wasn't able to find a similar bug report.

Potential fix:

The fix might be parsing the ngHref attribute and just observing when the {{hash}} changes and not the string around it. Here was the snippet that was adding the observers in booleanAttrs.js.

        attr.$observe(normalized, function(value) {
          if (!value)
             return;

          attr.$set(name, value);

          // <removed note about IE>

        });

@briangershon briangershon changed the title from ngHref prematurely generate href when string is included as part of the expression to ngHref prematurely adds href attribute when string is included as part of the expression Apr 3, 2014

@btford btford self-assigned this Apr 3, 2014

@btford btford added this to the Backlog milestone Apr 3, 2014

@btford

This comment has been minimized.

Show comment
Hide comment
@btford

btford Apr 3, 2014

Contributor

This is a known issue; I actually just wrote about it here: http://briantford.com/blog/angular-hacking-core.html

Contributor

btford commented Apr 3, 2014

This is a known issue; I actually just wrote about it here: http://briantford.com/blog/angular-hacking-core.html

@briangershon

This comment has been minimized.

Show comment
Hide comment
@briangershon

briangershon Apr 3, 2014

Thanks @btford for triaging, I'll check out your blog article.

Thanks @btford for triaging, I'll check out your blog article.

@btford

This comment has been minimized.

Show comment
Hide comment
@btford

btford Apr 3, 2014

Contributor

There's a significant pending change to $parse, but I'll work on it after that's in.

Contributor

btford commented Apr 3, 2014

There's a significant pending change to $parse, but I'll work on it after that's in.

@btford

This comment has been minimized.

Show comment
Hide comment
@btford

btford Apr 8, 2014

Contributor

@nicoabie thanks, but it's better to leave feedback on my post on the repo for my blog rather than here.

This thread is for discussing a solution.

Contributor

btford commented Apr 8, 2014

@nicoabie thanks, but it's better to leave feedback on my post on the repo for my blog rather than here.

This thread is for discussing a solution.

btford added a commit to btford/angular.js that referenced this issue Apr 28, 2014

fix(ngSrc, ngSrcset, ngHref): only interpolate if all expressions are…
… 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 #6984

btford added a commit to btford/angular.js that referenced this issue Apr 28, 2014

fix(ngSrc, ngSrcset, ngHref): only interpolate if all expressions are…
… 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 #6984

btford added a commit to btford/angular.js that referenced this issue Apr 28, 2014

fix(ngSrc, ngSrcset, ngHref): only interpolate if all expressions are…
… 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 #6984

btford added a commit to btford/angular.js that referenced this issue Apr 29, 2014

fix(ngSrc, ngSrcset, ngHref): only interpolate if all expressions are…
… 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 #6984

btford added a commit to btford/angular.js that referenced this issue Apr 30, 2014

fix(ngSrc, ngSrcset, ngHref): only interpolate if all expressions are…
… 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 #6984

btford added a commit to btford/angular.js that referenced this issue Apr 30, 2014

fix(ngSrc, ngSrcset, ngHref): only interpolate if all expressions are…
… 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 #6984

btford added a commit to btford/angular.js that referenced this issue Apr 30, 2014

fix(ngSrc, ngSrcset, ngHref): only interpolate if all expressions are…
… 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 #6984

btford added a commit to btford/angular.js that referenced this issue Apr 30, 2014

fix(ngSrc, ngSrcset, ngHref): only interpolate if all expressions are…
… 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 #6984

btford added a commit to btford/angular.js that referenced this issue May 1, 2014

fix(ngSrc, ngSrcset, ngHref): only interpolate if all expressions are…
… 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 #6984

btford added a commit to btford/angular.js that referenced this issue May 2, 2014

fix(ngSrc, ngSrcset): only interpolate if all expressions are 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` as well.

Closes #6984

btford added a commit to btford/angular.js that referenced this issue May 2, 2014

fix(ngSrc, ngSrcset): only interpolate if all expressions are 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` as well.

Closes #6984

btford added a commit to btford/angular.js that referenced this issue May 2, 2014

fix(ngSrc, ngSrcset): only interpolate if all expressions are 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` as well.

Closes #6984

btford added a commit to btford/angular.js that referenced this issue May 2, 2014

fix(ngSrc, ngSrcset): only interpolate if all expressions are 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` as well.

Closes #6984

btford added a commit to btford/angular.js that referenced this issue May 2, 2014

fix(ngSrc, ngSrcset): only interpolate if all expressions are 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` as well.

Closes #6984

btford added a commit to btford/angular.js that referenced this issue May 2, 2014

fix(ngSrc, ngSrcset): only interpolate if all expressions are 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` as well.

Closes #6984

@btford btford closed this in 8d18038 May 2, 2014

@briangershon

This comment has been minimized.

Show comment
Hide comment
@briangershon

briangershon May 2, 2014

Thanks @btford for your work on this!

Thanks @btford for your work on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment