Side navigation
#7853 closed bug (fixed)
Opened December 28, 2010 12:18PM UTC
Closed January 20, 2011 07:52PM UTC
`context` issue (incompatible change) when using DOM elements with $
Reported by: | T.J. Crowder | Owned by: | dmethvin |
---|---|---|---|
Priority: | high | Milestone: | 1.5 |
Component: | traversing | Version: | 1.4.4 |
Keywords: | Cc: | ||
Blocked by: | Blocking: |
Description
As of 1.4.x, using a DOM element with $()
apparently sets the jQuery object's context
property, which changes the behavior of functions like add
incompatibly with v1.3.x.
These two lines, which look like they should be identical, do very different things:
var a = $('#first').add('li');
var b = $(document.getElementById('first')).add('li');
The first line finds the element with the ID first
and adds any li
s in the document to it.
The second line finds the element with the ID first
and adds **only** li
s within that element.
Naturally this has a pretty big effect on event handlers, where $(this)
(with this
being a DOM element) is a very common use case.
Here's the the add
example code adapted to demonstrate the problem using 1.4.4 (1.4.0 also has the issue). 1.3.2 works fine.
This behavior is **very surprising**. People will expect the two lines above to do the same thing. I've seen people saying those two uses of $()
would be functionally the same all over the place, and of course they were the same prior to 1.4.0.
This looks and acts very much like a bug, but 1.4.0 came out nearly a year ago and the issue has existed from then through to 1.4.4 (the latest as of this writing), which makes me wonder if it's an intentional compatibility-breaking change. If so, it's unfortunate (IMHO, but I'm not involved with jQuery's development). It's also not mentioned in the release notes. In any case, if it remains, it needs **substantial** discussion in the documentation, not least the jQuery()
documentation, add
documentation, and anywhere else this.context
will have this impact. But unless it was a very carefully considered change, **strongly** recommend reverting it and dealing with the places that were relying on it (live
, I suspect) another way.
Details: The underlying cause of the behavior is that jQuery.fn.init
sets the context
to the DOM element (line 110 of v1.4.4; currently line 91 of core.js), and then add
uses this.context
(line 4,459 of jQuery.js v1.4.4; currently line 137 of traversing.js). It's probably used elsewhere as well.
Attachments (0)
Change History (9)
Changed December 28, 2010 03:02PM UTC by comment:1
component: | unfiled → traversing |
---|---|
priority: | undecided → high |
status: | new → open |
Changed December 28, 2010 03:15PM UTC by comment:2
Thanks Adam!
Changed December 28, 2010 03:49PM UTC by comment:3
Agreed, it seems like a bug. The fix seems pretty easy, remove the || this.context
inside the method and let it default to document
.
Changed December 28, 2010 03:50PM UTC by comment:4
owner: | → dmethvin |
---|---|
status: | open → assigned |
I'll grab this one. Thanks for the report!
Changed December 28, 2010 04:04PM UTC by comment:5
_comment0: | @Dave: No worries, and thanks. \ \ Yeah, the fix for `add` is easy, but I wonder if `.context` is being used elsewhere where it shouldn't be as well. I mean, obviously `$('#first')` and `$(document.getElementById('first'))` can't mean //exactly// the same thing, not if `live` is going to work with its current API. But a bit of an audit may be necessary to check that `.context` isn't being used elsewhere when it shouldn't be. I don't know the code well enough to do that. I see `.context` used in `closest`, for instance, but I don't know whether that use is correct or incorrect... → 1293552269711258 |
---|
@Dave: No worries, and thanks.
Yeah, the fix for add
is easy, but I wonder if .context
is being used elsewhere where it shouldn't be as well. I mean, obviously $('#first')
and $(document.getElementById('first'))
can't mean exactly the same thing, not if live
is going to work with its current API. But a bit of an audit may be necessary to check that .context
isn't being used elsewhere when it shouldn't be. I don't know the code well enough to do that. I see .context
used in closest
and a couple of other places, for instance, but I don't know whether that use is correct or incorrect...
Changed December 28, 2010 04:13PM UTC by comment:6
Amazingly, probably less than an hour after I figured out what was going on and filed this report, I ran into someone on StackOverflow running into it. Wow.
Changed December 29, 2010 02:50AM UTC by comment:7
Changed January 20, 2011 07:52PM UTC by comment:9
milestone: | 1.6 → 1.5 |
---|---|
resolution: | → fixed |
status: | assigned → closed |
Landed.
I was initially inclined to think that this was related to 7533, so I dropped the latest Git version into a jsbin as well, but the behaviour persists. I do agree that this is a bug - add(selector,context) was added in 1.4, but obviously the existing behaviour should not be broken.
Thanks for the report!