Bug Tracker

Ticket #7853 (closed bug: fixed)

Opened 4 years ago

Last modified 4 years ago

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

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 lis in the document to it.

The second line finds the element with the ID first and adds only lis 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.

Change History

comment:1 Changed 4 years ago by ajpiano

  • Priority changed from undecided to high
  • Status changed from new to open
  • Component changed from unfiled to traversing

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!

comment:2 Changed 4 years ago by T.J. Crowder

Thanks Adam!

comment:3 Changed 4 years ago by dmethvin

Agreed, it seems like a bug. The fix seems pretty easy, remove the || this.context inside the method and let it default to document.

comment:4 Changed 4 years ago by dmethvin

  • Owner set to dmethvin
  • Status changed from open to assigned

I'll grab this one. Thanks for the report!

comment:5 Changed 4 years ago by T.J. Crowder

@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...

Version 0, edited 4 years ago by T.J. Crowder (next)

comment:6 Changed 4 years ago by T.J. Crowder

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.

comment:8 Changed 4 years ago by ajpiano

#7904 is a duplicate of this ticket.

comment:9 Changed 4 years ago by john

  • Status changed from assigned to closed
  • Resolution set to fixed
  • Milestone changed from 1.6 to 1.5

Landed.

Note: See TracTickets for help on using tickets.