-
Notifications
You must be signed in to change notification settings - Fork 27.4k
fix(jqLite): pass in a dummy event with triggerHandler #2408
Conversation
This fails on both IE8 and IE9: http://ci.angularjs.org/job/angular.js-pete/77/ |
Previously, anchor elements could not be used with triggerHandler because triggerHandler passes null as the event, and any anchor element with an empty href automatically calls event.preventDefault(). Instead, pass a dummy event when using triggerHandler, similar to what full jQuery does. Modified from PR angular#2379.
Gah, IE. Thanks for pointing that out - should be fixed now. |
Hi, I think this is still failing: http://ci.angularjs.org/job/angular.js-pete/84/ |
The problem here is that in IE the dummy event is missing some important functions. JQuery builds its own events from scratch, wrapping real ones if necessary. See https://github.com/jquery/jquery/blob/master/src/event.js#L610. In our case we only want a dummy one. I have created this on my branch here: https://github.com/petebacondarwin/angular.js/compare/pr2408. This passes all CI tests. My only concern is that it adds quite a lot of bytes for such a small issue. Also there is a certain amount of overlap with the code in createEventHandler and perhaps this should all be consolidated into a single object/function? |
Sorry for the stall in responses! My immediate problem was fixed by the team deciding to just include jQuery in the whole app. I agree with your concern. Maybe the initial fix from PR #2379 could work? |
@juliemr - Well to be honest if this is the only place where we are worrying about this then we could make our dummy object really small with simply |
it('should pass a dummy event', function() { | ||
var element = jqLite('<a>poke</a>'), | ||
pokeSpy = jasmine.createSpy('poke'), | ||
event; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indentation is off
+1 for pete's suggestion |
landed as 5e556ac25845128b4137daa337cdbc575924dac9 |
Previously, anchor elements could not be used with triggerHandler because
triggerHandler passes null as the event, and any anchor element with an empty
href automatically calls event.preventDefault(). Instead, pass a dummy event
when using triggerHandler, similar to what full jQuery does. Modified from
PR #2379.