Fixed [1993] although it actually wasn't a bug in the core but rather a misunderstanding of how the extra function was supposed to work in jQuery.event.trigger(). That said, it seems more useful and robust for the code to work the way the ticket author thought it should work so this change was made.

Now, if anything is returned from the extra function it will overwrite the return value of the event handlers.  This should only effect custom events unless someone had an extra function that returned a value other than false which would have been ignored before.
This commit is contained in:
David Serduke 2007-12-03 21:41:10 +00:00
parent a73445bbc7
commit 66fbbec3bb
2 changed files with 53 additions and 21 deletions

View file

@ -171,8 +171,13 @@ jQuery.event = {
data.shift(); data.shift();
// Handle triggering of extra function // Handle triggering of extra function
if ( extra && extra.apply( element, data ) === false ) if ( extra ) {
val = false; // call the extra function and tack the current return value on the end for possible inspection
var ret = extra.apply( element, data.concat( val ) );
// if anything is returned, give it precedence and have it overwrite the previous value
if (ret !== undefined)
val = ret;
}
// Trigger the native events (except for clicks on links) // Trigger the native events (except for clicks on links)
if ( fn && donative !== false && val !== false && !(jQuery.nodeName(element, 'a') && type == "click") ) { if ( fn && donative !== false && val !== false && !(jQuery.nodeName(element, 'a') && type == "click") ) {

View file

@ -48,34 +48,34 @@ test("bind()", function() {
reset(); reset();
$("#firstp").bind("click",function(e){ $("#firstp").bind("click",function(e){
ok(true, "Normal click triggered"); ok(true, "Normal click triggered");
}); });
$("#firstp").bind("click.test",function(e){ $("#firstp").bind("click.test",function(e){
ok(true, "Namespaced click triggered"); ok(true, "Namespaced click triggered");
}); });
// Trigger both bound fn (2) // Trigger both bound fn (2)
$("#firstp").trigger("click"); $("#firstp").trigger("click");
// Trigger one bound fn (1) // Trigger one bound fn (1)
$("#firstp").trigger("click.test"); $("#firstp").trigger("click.test");
// Remove only the one fn // Remove only the one fn
$("#firstp").unbind("click.test"); $("#firstp").unbind("click.test");
// Trigger the remaining fn (1) // Trigger the remaining fn (1)
$("#firstp").trigger("click"); $("#firstp").trigger("click");
}); });
test("click()", function() { test("click()", function() {
expect(4); expect(4);
$('<li><a href="#">Change location</a></li>').prependTo('#firstUL').find('a').bind('click', function() { $('<li><a href="#">Change location</a></li>').prependTo('#firstUL').find('a').bind('click', function() {
var close = $('spanx', this); // same with $(this).find('span'); var close = $('spanx', this); // same with $(this).find('span');
ok( close.length == 0, "Context element does not exist, length must be zero" ); ok( close.length == 0, "Context element does not exist, length must be zero" );
ok( !close[0], "Context element does not exist, direct access to element must return undefined" ); ok( !close[0], "Context element does not exist, direct access to element must return undefined" );
return false; return false;
}).click(); }).click();
$("#check1").click(function() { $("#check1").click(function() {
@ -116,7 +116,7 @@ test("unbind(event)", function() {
}); });
test("trigger(event, [data], [fn])", function() { test("trigger(event, [data], [fn])", function() {
expect(40); expect(66);
var handler = function(event, a, b, c) { var handler = function(event, a, b, c) {
equals( event.type, "click", "check passed data" ); equals( event.type, "click", "check passed data" );
@ -130,7 +130,22 @@ test("trigger(event, [data], [fn])", function() {
equals( a, 1, "check passed data" ); equals( a, 1, "check passed data" );
equals( b, "2", "check passed data" ); equals( b, "2", "check passed data" );
equals( c, "abc", "check passed data" ); equals( c, "abc", "check passed data" );
return "test2"; return false;
};
var handler3 = function(a, b, c, v) {
equals( a, 1, "check passed data" );
equals( b, "2", "check passed data" );
equals( c, "abc", "check passed data" );
equals( v, "test", "check current value" );
return "newVal";
};
var handler4 = function(a, b, c, v) {
equals( a, 1, "check passed data" );
equals( b, "2", "check passed data" );
equals( c, "abc", "check passed data" );
equals( v, "test", "check current value" );
}; };
// Simulate a "native" click // Simulate a "native" click
@ -143,21 +158,25 @@ test("trigger(event, [data], [fn])", function() {
$("#firstp").bind("click", handler).trigger("click", [1, "2", "abc"]); $("#firstp").bind("click", handler).trigger("click", [1, "2", "abc"]);
// Triggers handlers, native, and extra fn // Triggers handlers, native, and extra fn
// Triggers 8 // Triggers 9
$("#firstp").trigger("click", [1, "2", "abc"], handler2); $("#firstp").trigger("click", [1, "2", "abc"], handler4);
// Simulate a "native" click // Simulate a "native" click
$("#firstp")[0].click = function(){ $("#firstp")[0].click = function(){
ok( false, "Native call was triggered" ); ok( false, "Native call was triggered" );
}; };
// Triggers handlers, native, and extra fn
// Triggers 7
$("#firstp").trigger("click", [1, "2", "abc"], handler2);
// Trigger only the handlers (no native) // Trigger only the handlers (no native)
// Triggers 5 // Triggers 5
equals( $("#firstp").triggerHandler("click", [1, "2", "abc"]), "test", "Verify handler response" ); equals( $("#firstp").triggerHandler("click", [1, "2", "abc"]), "test", "Verify handler response" );
// Trigger only the handlers (no native) and extra fn // Trigger only the handlers (no native) and extra fn
// Triggers 8 // Triggers 8
equals( $("#firstp").triggerHandler("click", [1, "2", "abc"], handler2), "test", "Verify handler response" ); equals( $("#firstp").triggerHandler("click", [1, "2", "abc"], handler2), false, "Verify handler response" );
// Build fake click event to pass in // Build fake click event to pass in
var eventObj = jQuery.event.fix({ type: "foo", target: document.body }); var eventObj = jQuery.event.fix({ type: "foo", target: document.body });
@ -169,6 +188,14 @@ test("trigger(event, [data], [fn])", function() {
// Trigger only the handlers (no native) and extra fn, with external event obj // Trigger only the handlers (no native) and extra fn, with external event obj
// Triggers 9 // Triggers 9
equals( $("#firstp").triggerHandler("click", [eventObj, 1, "2", "abc"], handler), "test", "Verify handler response" ); equals( $("#firstp").triggerHandler("click", [eventObj, 1, "2", "abc"], handler), "test", "Verify handler response" );
// have the extra handler override the return
// Triggers 9
equals( $("#firstp").triggerHandler("click", [1, "2", "abc"], handler3), "newVal", "Verify triggerHandler return is overwritten by extra function" );
// have the extra handler leave the return value alone
// Triggers 9
equals( $("#firstp").triggerHandler("click", [1, "2", "abc"], handler4), "test", "Verify triggerHandler return is not overwritten by extra function" );
}); });
test("toggle(Function, Function)", function() { test("toggle(Function, Function)", function() {