Bug Tracker

Modify

Ticket #2811 (closed bug: fixed)

Opened 5 years ago

Last modified 5 years ago

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:
Blocking: Blocked by:

Description (last modified by flesler) (diff)

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

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

Change History

comment:1 Changed 5 years ago by flesler

  • Status changed from new to assigned

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 5 years ago by flesler

Changed 5 years ago by flesler

Tests that can now pass

comment:2 Changed 5 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 5 years ago by flesler

  • Description modified (diff)

comment:4 Changed 5 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 5 years ago by joern

There is nothing fun as the form element in IE.

Agreed with Scott!

comment:6 Changed 5 years ago by flesler

  • Status changed from assigned to closed
  • Resolution set to fixed

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

Please follow the  bug reporting guidlines and use  jsFiddle when providing test cases and demonstrations instead of pasting the code in the ticket.

View

Add a comment

Modify Ticket

Action
as closed
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.