Skip to content
Permalink
Browse files

Fixed a loop that only worked in webkit.

  • Loading branch information...
jaubourg
jaubourg committed Dec 20, 2010
1 parent 88d97de commit 9e3f053c6568863251da20eba713d05f2bbdf9f0
Showing with 8 additions and 6 deletions.
  1. +8 −6 src/core.js
@@ -800,12 +800,14 @@ jQuery.extend({
deferred = {

// then( f1, f2, ...)
then: function() {
then: function then() {

if ( ! cancelled ) {

var args = arguments,
i,
length,
elem,
type,
_fired;

@@ -814,13 +816,13 @@ jQuery.extend({
fired = 0;
}

for ( i in args ) {
i = args[ i ];
type = jQuery.type( i );
for ( i = 0, length = args.length ; i < length ; i++ ) {
elem = args[ i ];
type = jQuery.type( elem );
if ( type === "array" ) {
this.then.apply( this , i );
then.apply( this , elem );
} else if ( type === "function" ) {
callbacks.push( i );
callbacks.push( elem );
}
}

2 comments on commit 9e3f053

@davidmurdoch

This comment has been minimized.

Copy link
Contributor

davidmurdoch replied Dec 21, 2010

Won't the named function expression then "leak" into _deferred's scope in IE < 9? I think then needs to be nulled probably somewhere around line 888 (var then = null;) so JScript's GC can do its job properly. (http://kangax.github.com/nfe/#jscript-bugs).

Also, this type of NFE might break in Safari 2.x (http://kangax.github.com/nfe/#safari-bug).

Or because the NFE's purpose isn't for debugging the function itself why not just use deferred.then.apply( this, elem ); and get rid of the NFE altogether (no other parts of jQuery have them, right?). If you absolutely must keep the then.apply( this, elem ); line as-is declare then as a function declaration in _deferred's body then set it up as: deferred = { then: then, ... };.

@jaubourg

This comment has been minimized.

Copy link
Member

jaubourg replied Dec 21, 2010

Better safe than sorry: c810c62

Thanks for the warning :)

Please sign in to comment.
You can’t perform that action at this time.