fix(ngHref): remove attribute when empty value instead of ignoring

[shahata]

this will allow updating the values of ng-src, ng-srcset, ng-href back to empty state while preserving the intended behavior of not setting the attribute to an empty string to prevent behavior like trying to download the current page as an image or creating a link to the current page.

Fixes #2755

[mary-poppins]

Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.

• Uses the issue template (#6986)

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

[shahata]

The idea is that we remove the src/srcset/href attribute when value is empty. This works well on IE8 for both images and href. Please advise if I should write specific tests for msie.

[caitp]

Despite correctly removing the href attribute, I think it would be better to ensure that the original src attribute behaviour continues to behave as expected

[shahata]

@caitp I'm not sure I understand what you mean. Can you elaborate?

[caitp]

the original patch is meant to just avoid changing the src attribute if the interpolated value is blank. It may be better to change this so that it leaves a blank src value alone.

[shahata]

The only difference in my implementation is that when the source becomes blank I will remove the src attribute instead of leaving it with its current value. This means that the current image will disappear, while in the previous implementation it will remain the same.

The way I see it, the previous implementation wanted to avoid downloading of image with blank url, but solved it by creating a behavior which is not intuitive, while the new implementation solved the download of blank url with behavior that makes more sense.

I could change this to exclude src attribute so it will only happen if it is href or srcset, but I like it that the implementation is consistent. What do you think?

[caitp]

I believe the reason they don't want to change the URL is because they don't want to see FOUC-ish behaviour with the image beginning to load and then being removed, possibly multiple times

[shahata]

Not sure I understand when the value can change from non-blank to blank multiple times, but I pushed an amended commit which makes this work only for href. Looks good now?

[shahata]

@caitp - I've rebased this.

[hsablonniere]

Where are we on this one ? I have the same problem :-(

[ajoslin]

Also encountering this problem in Ionic. Any updates? Anything that needs to be done to help it get merged?

[btford]

This LGTM. @caitp shall we merge this?

[caitp]

yes, sure. My source tree is a bit busy right now (promise refactoring), so it will be a while before I can check it in. You should do it!

[btford]

✨ consider it done ✨

[ajoslin]

Awesome! Thanks.

[btford]

Landed as 469ea33. Thanks @shahata!

[pholly]

Now anchor tags with href="" and ng-href="" remove the href attribute so browsers styles them differently (no pointer, for example in Chrome).