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

fix(htmlAnchorDirective): remove event listener if target is not element #10849

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions 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) {
// If a different element was clicked, ignore it.
if (element[0] !== event.target) return;
Copy link

Choose a reason for hiding this comment

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

I think this line caused regression. preventDefault() is below and not called anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this code hasn't been released yet, but thanks for testing out the cutting edge, i'll push a fix tonight

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but just to make sure, you aren't using "cutting edge" in production right? that would be a bad idea.

Copy link

Choose a reason for hiding this comment

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

You are right, I'm not using edge. Just tested my PR
#7995 to be working after rebase.

2015-01-26 14:37 GMT+10:00 ⭐caitp⭐ notifications@github.com:

In src/ng/directive/a.js
#10849 (comment):

     // 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) {
  •      // If a different element was clicked, ignore it.
    
  •      if (element[0] !== event.target) return;
    

but just to make sure, you aren't using "cutting edge" in production
right? that would be a bad idea.


Reply to this email directly or view it on GitHub
https://github.com/angular/angular.js/pull/10849/files#r23512554.

Пашин Анатолий,
эникейщик.


// 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