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

Commit 45a52d2

Browse files
committed
fix(htmlAnchorDirective): remove event listener if target is not element
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 #4262
1 parent cea8e75 commit 45a52d2

File tree

2 files changed

+61
-3
lines changed

2 files changed

+61
-3
lines changed

src/ng/directive/a.js

+8-3
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,14 @@ var htmlAnchorDirective = valueFn({
2121
// SVGAElement does not use the href attribute, but rather the 'xlinkHref' attribute.
2222
var href = toString.call(element.prop('href')) === '[object SVGAnimatedString]' ?
2323
'xlink:href' : 'href';
24-
element.on('click', function(event) {
25-
// if we have no href url, then don't navigate anywhere.
26-
if (!element.attr(href)) {
24+
element.on('click', function clickHandler(event) {
25+
if (element[0] !== event.target) {
26+
// if the clicked element is not the directive element, stop listening.
27+
// (the element was probably replaced).
28+
element.off('click', clickHandler);
29+
return;
30+
} else if (!element.attr(href)) {
31+
// if we have no href url, then don't navigate anywhere.
2732
event.preventDefault();
2833
}
2934
});

test/ng/directive/aSpec.js

+53
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,25 @@
33
describe('a', function() {
44
var element, $compile, $rootScope;
55

6+
beforeEach(module(function($compileProvider) {
7+
$compileProvider.
8+
directive('linkTo', valueFn({
9+
restrict: 'A',
10+
template: '<div class="my-link"><a href="{{destination}}">{{destination}}</a></div>',
11+
replace: true,
12+
scope: {
13+
destination: '@linkTo'
14+
}
15+
})).
16+
directive('linkNot', valueFn({
17+
restrict: 'A',
18+
template: '<div class="my-link"><a href>{{destination}}</a></div>',
19+
replace: true,
20+
scope: {
21+
destination: '@linkNot'
22+
}
23+
}));
24+
}));
625

726
beforeEach(inject(function(_$compile_, _$rootScope_) {
827
$compile = _$compile_;
@@ -76,6 +95,40 @@ describe('a', function() {
7695
});
7796

7897

98+
it('should not preventDefault if anchor element is replaced with href-containing element', function() {
99+
spyOn(jqLite.prototype, 'on').andCallThrough();
100+
element = $compile('<a link-to="https://www.google.com">')($rootScope);
101+
$rootScope.$digest();
102+
103+
var child = element.children('a');
104+
var preventDefault = jasmine.createSpy('preventDefault');
105+
106+
child.triggerHandler({
107+
type: 'click',
108+
preventDefault: preventDefault
109+
});
110+
111+
expect(preventDefault).not.toHaveBeenCalled();
112+
});
113+
114+
115+
it('should preventDefault if anchor element is replaced with element without href attribute', function() {
116+
spyOn(jqLite.prototype, 'on').andCallThrough();
117+
element = $compile('<a link-not="https://www.google.com">')($rootScope);
118+
$rootScope.$digest();
119+
120+
var child = element.children('a');
121+
var preventDefault = jasmine.createSpy('preventDefault');
122+
123+
child.triggerHandler({
124+
type: 'click',
125+
preventDefault: preventDefault
126+
});
127+
128+
expect(preventDefault).toHaveBeenCalled();
129+
});
130+
131+
79132
if (isDefined(window.SVGElement)) {
80133
describe('SVGAElement', function() {
81134
it('should prevent default action to be executed when href is empty', function() {

0 commit comments

Comments
 (0)