Skip to content

Commit

Permalink
fix(htmlAnchorDirective): remove event listener if target is not element
Browse files Browse the repository at this point in the history
Previously, when an `a` tag element used a directive with a replacing template, and did not include an `href` or `name` attribute
before linkage, the anchor directive would always prevent default.

Now, the anchor directive will cancel its event listener if the linked element is not the same as the target element of the event.

Closes angular#4262
  • Loading branch information
caitp committed Jan 23, 2015
1 parent cea8e75 commit 4323da9
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 1 deletion.
8 changes: 7 additions & 1 deletion src/ng/directive/a.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,16 @@ var htmlAnchorDirective = valueFn({
compile: function(element, attr) {
if (!attr.href && !attr.xlinkHref && !attr.name) {
return function(scope, element) {
// If the linked element is not an anchor tag anymore, do nothing
if (element[0].nodeName.toLowerCase() !== 'a') return;

// SVGAElement does not use the href attribute, but rather the 'xlinkHref' attribute.
var href = toString.call(element.prop('href')) === '[object SVGAnimatedString]' ?
'xlink:href' : 'href';
element.on('click', function(event) {
element.on('click', function clickHandler(event) {
// If a different element was clicked, ignore it.
if (element[0] !== event.target) return;

// if we have no href url, then don't navigate anywhere.
if (!element.attr(href)) {
event.preventDefault();
Expand Down
53 changes: 53 additions & 0 deletions test/ng/directive/aSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,25 @@
describe('a', function() {
var element, $compile, $rootScope;

beforeEach(module(function($compileProvider) {
$compileProvider.
directive('linkTo', valueFn({
restrict: 'A',
template: '<div class="my-link"><a href="{{destination}}">{{destination}}</a></div>',
replace: true,
scope: {
destination: '@linkTo'
}
})).
directive('linkNot', valueFn({
restrict: 'A',
template: '<div class="my-link"><a href>{{destination}}</a></div>',
replace: true,
scope: {
destination: '@linkNot'
}
}));
}));

beforeEach(inject(function(_$compile_, _$rootScope_) {
$compile = _$compile_;
Expand Down Expand Up @@ -76,6 +95,40 @@ describe('a', function() {
});


it('should not preventDefault if anchor element is replaced with href-containing element', function() {
spyOn(jqLite.prototype, 'on').andCallThrough();
element = $compile('<a link-to="https://www.google.com">')($rootScope);
$rootScope.$digest();

var child = element.children('a');
var preventDefault = jasmine.createSpy('preventDefault');

child.triggerHandler({
type: 'click',
preventDefault: preventDefault
});

expect(preventDefault).not.toHaveBeenCalled();
});


it('should preventDefault if anchor element is replaced with element without href attribute', function() {
spyOn(jqLite.prototype, 'on').andCallThrough();
element = $compile('<a link-not="https://www.google.com">')($rootScope);
$rootScope.$digest();

var child = element.children('a');
var preventDefault = jasmine.createSpy('preventDefault');

child.triggerHandler({
type: 'click',
preventDefault: preventDefault
});

expect(preventDefault).toHaveBeenCalled();
});


if (isDefined(window.SVGElement)) {
describe('SVGAElement', function() {
it('should prevent default action to be executed when href is empty', function() {
Expand Down

0 comments on commit 4323da9

Please sign in to comment.