Bug Tracker

Opened 12 years ago

Closed 11 years ago

Last modified 9 years ago

#2827 closed enhancement (fixed)

$.each Treats Functions as Arrays

Reported by: dschoon Owned by: flesler
Priority: minor Milestone: 1.4
Component: core Version: 1.3.2
Keywords: Cc: dschoon
Blocked by: Blocking:

Description

Calling $.each on a function will result in it iterating over [0..arity], not the properties of the function object.

This is because $.each checks whether obj.length is defined, rather than checking whether the object is an array. In the case of a function object, Function.length will be defined, and it will be the arity of the function. This also screws user-defined types which have a length property.

Boo!

Attachments (2)

dollar-each.diff (1.3 KB) - added by dschoon 12 years ago.
t2827.diff (700 bytes) - added by micah 11 years ago.
Patch for #2827

Download all attachments as: .zip

Change History (12)

Changed 12 years ago by dschoon

Attachment: dollar-each.diff added

comment:1 Changed 12 years ago by dschoon

Patch attached. I guess I should have checked whether I have check-in privileges by default, but I assume not.

comment:2 Changed 12 years ago by flesler

Resolution: wontfix
Status: newclosed

I can tell you in advance, what a core developer will say: this function is internal, is the most used function all over the framework, so if this behavior (iterating functions) is not required by the core, then it won't be changed.

Also, this patch doesn't support any array-like (arguments, nodelist, jQuery object, etc).

I guess I should have checked whether I have check-in privileges by default, but I assume not.

There's no permissions system integrated into the SVN. But only those allowed can "legally" commit.

comment:3 Changed 12 years ago by dschoon

Internal? Really? http://docs.jquery.com/Core/each#callback

I mean, if it's in the docs, it seems pretty public to me.

comment:4 Changed 12 years ago by dschoon

Also, legit criticism for array-like objects; I guess the only technically correct solution would be to special-case for Functions.

comment:5 Changed 12 years ago by flesler

You're confusing $.each and $().each.

$.each is a generic iterator, it receives 2 arguments: collection and callback

$().each ($.fn.each) is a special use of $.each, it iterates over the elements of a jQuery collection. It only receives the callback, and the calling collection is used.

The function mentioned in the docs, refers to the callback. Functions aren't supported.

comment:6 Changed 11 years ago by micah

Resolution: wontfix
Status: closedreopened

Reopening this, as it still exists and IMO is indeed a valid bug.

In http://dev.jquery.com/browser/tags/1.3.2/src/core.js#L662 $.each checks for length === undefined, which breaks in this use case:

var someFunction = function(){};
$.each(someFunction, function(key){
    alert(key);
});

The docs say "Other objects are iterated via their named properties.", so you'd expect that code to iterate over the built-in properties and methods of that instance of Function, but instead it tries to loop through properties named [0...arity], which don't exist.

The fix is pretty painless. Change the two offending lines from:

if ( length === undefined ) {

to:

if ( length === undefined || object.constructor == Function ) {

...et voila, $.each should hit the for...in condition that works on functions.

I only added the specific exception for Function because I can't think of anything else off the top of my head that allows for...in but doesn't currently work with $.each.

Changed 11 years ago by micah

Attachment: t2827.diff added

Patch for #2827

comment:7 Changed 11 years ago by flesler

Cc: dschoon added
Milestone: 1.2.41.3.3
Owner: set to flesler
Status: reopenednew
Type: bugenhancement
Version: 1.2.31.3.2

comment:8 Changed 11 years ago by flesler

Status: newassigned

comment:9 Changed 11 years ago by flesler

Resolution: fixed
Status: assignedclosed

Fixed at [6410].

comment:12 Changed 9 years ago by addyosmani

#5002 is a duplicate of this ticket.

Note: See TracTickets for help on using tickets.