Bug Tracker

Opened 12 years ago

Closed 12 years ago

#2811 closed bug (fixed)

2 bugs on jQuery.fn.add

Reported by: flesler Owned by: flesler
Priority: major Milestone: 1.2.4
Component: core Version: 1.2.3
Keywords: add duplicated unique window form Cc:
Blocked by: Blocking:

Description (last modified by flesler)

As mentioned here, $.fn.add is not calling $.unique, thus elements can get duplicated.

Also, the array-like detection is very naive, and will fail when receiving a form, or the window if iframes are included.

That's why I change it so that it uses makeArray, which does smarter sniffing on array-like.

This also makes the code shorter.

Attachments (2)

add.diff (673 bytes) - added by flesler 12 years ago.
testrunner-add.diff (1.1 KB) - added by flesler 12 years ago.
Tests that can now pass

Download all attachments as: .zip

Change History (8)

comment:1 Changed 12 years ago by flesler

Status: newassigned

UPDATE:

I just got into the scariest IE quirk. Just realized that form == form.elements. They're literally the same, can't differentiate them. I decided (to keep this working as it was) that $(form) will consider it a form, but $.makeArray(form) will generate an array of elements, this is the behavior of $.fn.add.

In short, $.fn.add does a naive check, will mess up with $().add(window) and any other thing with length.

Changed 12 years ago by flesler

Attachment: add.diff added

Changed 12 years ago by flesler

Attachment: testrunner-add.diff added

Tests that can now pass

comment:2 Changed 12 years ago by flesler

Noticed that one test of .html() fails on IE with this patch, because the created element gets the expando, so the resulting .html() includes it and is different than expected.

This shouldn't be relevant.

In conclusion, do we want to call .unique() inside .add() ??

comment:3 Changed 12 years ago by flesler

Description: modified (diff)

comment:4 Changed 12 years ago by scott.gonzal

This change looks good to me. makeArray should be used wherever possible and the added call to unique seems like expected behavior.

comment:5 Changed 12 years ago by joern

There is nothing fun as the form element in IE.

Agreed with Scott!

comment:6 Changed 12 years ago by flesler

Resolution: fixed
Status: assignedclosed

Applied at [5503] and extra tests at [5504].

Note: See TracTickets for help on using tickets.