Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Commit

Permalink
fix(a): workaround IE bug affecting mailto urls
Browse files Browse the repository at this point in the history
Apparently there is a really weird bug in IE6-8 that causes anchor textContent
to be reset with href content when both contain @ symbol.

Inserting a bogus comment node into all anchor elements in IE works around this
browser bug.

I'm fixing the issue via directive because that way we'll fix it for jQuery as
well.

I fixed an e2e test too because it was incorrect.

Closes #1949
  • Loading branch information
IgorMinar committed Feb 15, 2013
1 parent 1ace5eb commit 37e8b12
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 9 deletions.
18 changes: 14 additions & 4 deletions src/ng/directive/a.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,20 @@
var htmlAnchorDirective = valueFn({
restrict: 'E',
compile: function(element, attr) {
// turn <a href ng-click="..">link</a> into a link in IE
// but only if it doesn't have name attribute, in which case it's an anchor
if (!attr.href) {
attr.$set('href', '');

if (msie <= 8) {

// turn <a href ng-click="..">link</a> into a stylable link in IE
// but only if it doesn't have name attribute, in which case it's an anchor
if (!attr.href && !attr.name) {
attr.$set('href', '');
}

// add a comment node to anchors to workaround IE bug that causes element content to be reset
// to new attribute content if attribute is updated with value containing @ and element also
// contains value with @
// see issue #1949
element.append(document.createComment('IE fix'));
}

return function(scope, element) {
Expand Down
2 changes: 1 addition & 1 deletion src/ng/directive/booleanAttrs.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
it('should execute ng-click but not reload when no href but name specified', function() {
element('#link-5').click();
expect(input('value').val()).toEqual('5');
expect(element('#link-5').attr('href')).toBe('');
expect(element('#link-5').attr('href')).toBe(undefined);
});
it('should only change url when only ng-href', function() {
Expand Down
23 changes: 19 additions & 4 deletions test/ng/directive/aSpec.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,21 @@
'use strict';

describe('a', function() {
var element;
var element, $compile, $rootScope;


beforeEach(inject(function(_$compile_, _$rootScope_) {
$compile = _$compile_;
$rootScope = _$rootScope_;
}));


afterEach(function(){
dealoc(element);
});


it('should prevent default action to be executed when href is empty',
inject(function($rootScope, $compile) {
it('should prevent default action to be executed when href is empty', function() {
var orgLocation = document.location.href,
preventDefaultCalled = false,
event;
Expand Down Expand Up @@ -42,5 +47,15 @@ describe('a', function() {
}

expect(document.location.href).toEqual(orgLocation);
}));
});


it('should prevent IE for changing text content when setting attribute', function() {
// see issue #1949
element = jqLite('<a href="">hello@you</a>');
$compile(element);
element.attr('href', 'bye@me');

expect(element.text()).toBe('hello@you');
});
});

2 comments on commit 37e8b12

@Camuensei
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before updating angular to this version, I used the fact that an empty href used in combination with ng-click allowed all browsers to show the link as clickable. Now, I have to simulate that behaviour through css... Is that the wanted behaviour ?

@MattPLavoie
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before this change it seems like the href attribute was being set on all anchors where the code's intention was to set it on all anchors for browsers IE 8 and older. I kind of assumed you guys were doing this on purpose because it was nice to not have to put an empty href on every a tag in order to have the element be focus-able, but since upgrading from 1.0.4 all of my anchors are now behaving in not the desired way.

@IgorMinar would you mind speaking at all to this, maybe explaining this change an if it the consequences of it were intentional. Are there any anchors out there that people don't want to have href attributes on, or at least the behavior that comes with that?

Please sign in to comment.