11

The documentation for jquery's closest says the following:

.closest( selector [, context ] )
...
context
Type: Element
A DOM element within which a matching element may be found. If no context is passed in then the context of the jQuery set will be used instead.

As I understand it, the bolded text means that the two statements should be equivalent:

set.closest("a");

set.closest("a", set.context);

where set is some jquery set.

However, this does not seem to be the case:

var context = $("#inner")[0];
var set = $("#el", context);

// the set's context is correctly the "inner" element
set.text("context: " + set.context.id);

// if the set's context is used, this closest should match nothing, but it matches and sets the color
set.closest("#outer").css("color", "red");

// with the context explicitly set, the "outer" is not found and no background color is set
set.closest("#outer", set.context).css("background-color", "blue");
#outer{
  width: 100px;
  height: 100px;
  border: 1px solid black;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.11.1/jquery.min.js"></script>
<div id="outer">
  <div id="inner">
    <div id="el"></div>
  </div>
</div>

As you can see, when no context is explicitly set, the set's context does not seem to be used as the #outer element is found by closest. When explicitly set, the #outer is correctly not found.

Is the documentation just incorrect or am I missing something?

mplungjan
  • 169,008
  • 28
  • 173
  • 236
James Montagne
  • 77,516
  • 14
  • 110
  • 130
  • 1
    I guess the documentation is wrong. It doesn't look like the context of the jQuery set is used if the context isn't specified. – Josh Crozier Jan 03 '16 at 18:37
  • Why you did it like `var set = $("#el", context);` ? As per doc reading what I am getting is, `closest()` try to match the `selector` expression up the ancestor chains, till it finds a match. Now the second argument the method `closest()` is just to give it the a limit till that point it will keep hunting for a match. And if no second argument it will be decided by the Jquery where `body` might be the last parent. – Arup Rakshit Jan 03 '16 at 18:43
  • So your code looks like doesn't the proper example to go against the Jquery doc. – Arup Rakshit Jan 03 '16 at 18:47
  • It's somewhat confusing because jQuery's `closest()` doesn't always use the context, it uses a regex to determine if a context is needed, like this `/^[\x20\t\r\n\f]*[>+~]|:(even|odd|eq|gt|lt|nth|first|last)(?:\([\x20\t\r\n\f]*((?:-\d)?\d*)[\x20\t\r\n\f]*\)|)(?=[^-]|$)/i`, so just changing it to `set.closest("#outer:first")` makes it use the context, and no longer match the element, while `$('#el').closest("#outer:first")` still matches the element like it should. – adeneo Jan 03 '16 at 19:11
  • I would prefer using `parents( selector )` for parent element and in your code simply use id selector – iamawebgeek Jan 03 '16 at 19:13

1 Answers1

5

This is clearly a bug, and not how it was intended to work.

The source for closest() is

function (selectors, context) {
    var cur, 
        i = 0,
        l = this.length,
        matched = [],
        pos = rneedsContext.test(selectors) || typeof selectors !== "string" 
           ? 
           jQuery(selectors, context || this.context) 
           : 
           0;

    for (; i < l; i++) {
        for (cur = this[i]; cur && cur !== context; cur = cur.parentNode) {
            // Always skip document fragments
            if (cur.nodeType < 11 && (pos ? pos.index(cur) > -1 :

            // Don't pass non-elements to Sizzle
            cur.nodeType === 1 && jQuery.find.matchesSelector(cur, selectors))) {

                matched.push(cur);
                break;
            }
        }
    }

    return this.pushStack(matched.length > 1 ? jQuery.unique(matched) : matched);
}

What's notable is the way pos is defined, it's the collection to be searched for a closest parent element, and rneedsContext is the regex

/^[\x20\t\r\n\f]*[>+~]|:(even|odd|eq|gt|lt|nth|first|last)(?:\([\x20\t\r\n\f]*((?:-\d)?\d*)[\x20\t\r\n\f]*\)|)(?=[^-]|$)/i

If the passed in selector doesn't match that regex, no context is used whatsoever, pos would equal 0, and the check for cur in that collection is just skipped all together, which seems mighty strange.

A quick test shows

var reg = /^[\x20\t\r\n\f]*[>+~]|:(even|odd|eq|gt|lt|nth|first|last)(?:\([\x20\t\r\n\f]*((?:-\d)?\d*)[\x20\t\r\n\f]*\)|)(?=[^-]|$)/i;

reg.test('#outer');       // false, no context used
reg.test('#outer:first'); // true, context used
reg.test('#outer:eq(0)'); // true, context used

So if you add a pseudo selector, it suddenly uses the context ?

I doubt this is what was intended, and it seems like a strange thing to do, and it surely doesn't do what the documentation says it should do.

adeneo
  • 312,895
  • 29
  • 395
  • 388
  • _"The source for closest() is"_ Which version is source at post ? See http://stackoverflow.com/questions/34580013/jquery-closest-default-context/34580530#comment56906623_34580530 – guest271314 Jan 03 '16 at 19:45
  • @guest271314 - that's from 1.11.2, but it's the same for all somewhat current versions – adeneo Jan 03 '16 at 19:46
  • Tried both jsfiddles ? Issue appear to have been addressed at lastest ? Though `.text()` not set ? Or, is jsfiddle not loading latest ? Using jQuery object for `DOM` element sets `background-color` `set.closest("#outer", $(set.context)).css("background-color", "blue");` – guest271314 Jan 03 '16 at 19:47
  • 4
    Looking at the release notes for 2.1.4 there's no changes to `closest`, and looking at the source for 2.1.3 `closest`, and specifically the `pos` variable, is defined the same way as in the last ten versions of jQuery. Testing with 2.1.4 in jsFiddle seems to still have the same issues as well, so I think this is a bug, hats of to James for finding it, it is a strange one. – adeneo Jan 03 '16 at 19:52
  • Why does using jQuery object set `background-color` https://jsfiddle.net/qeb26cox/ ? – guest271314 Jan 03 '16 at 19:53
  • Thanks adeneo, I had a really quick look at the source last night but hadn't gotten to the point of looking at what `rneedsContext` was actually doing. Now I'm curious what they're actually trying to do there. I might skim the overall jquery source to see where else they use it to see why it's needed. – James Montagne Jan 03 '16 at 19:54
  • @JamesMontagne - it sure seems strange that they use a regex like that to determine wether or not to use context, it surely can't be intended that if you add pseudoselectors like `;eq()`, `:first` etc. it suddenly works as documented, someone did something strange, and noone has spotted it in years ? – adeneo Jan 03 '16 at 19:57
  • The only thing I could figure, is that it would probably be even stranger if `closest()` used the context from the passed in collection (the original context), as that would seem strange to most people when they do `$('#inner', '#outer').closest('body')` and find nothing, because the search is suddenly limited to `#outer`. – adeneo Jan 03 '16 at 20:00
  • @adeneo actually, looking at it, the use of context is actually in the loop condition whereas `pos` is doing something else entirely. It seems like some additional check is needed when that regex matches (and maybe rightfully so) but that the loop condition is the actual bug – James Montagne Jan 03 '16 at 20:00
  • Seems almost like `matchesSelector` doesn't work for these matched by the regex so it gets the set first and checks if it is contained within. If it doesn't match the regex then it uses `matchesSelector`. – James Montagne Jan 03 '16 at 20:02
  • I think that if `pos` is **not** `0`, or in other words if `pos` is a jQuery collection, it uses `pos.index(cur) > -1` to determine wether or not to push the element to the array. Of course, that checks to see if `cur` is inside `pos`, like you'd expect when passing in context. If `pos` is **not** a jQuery collection, but `0`, the regex test failed, and the condition checks that `cur` is an element, and that it matches the selector passed to `closest()`, before adding `cur` to the array, all done without any checks for context. I think the condition works, as long as `pos` is set correctly. – adeneo Jan 03 '16 at 20:07
  • It looks like `matchesSelector` tries to use the browser's native `matches`. Of course `matches` won't work with things like `:gt` which are created by `jquery` so `matchesSelector` cant' be used if that is part of the selector. So the `pos` thing is for those edge cases. The edge case code uses context as documented, the simple case (browser supported selector) doesn't – James Montagne Jan 03 '16 at 20:09
  • But `matchesSelector` just does `matchesSelector(element, selector)` and returns true if the element passed in can be matched by the selector, there's no context used there, neither from the original collection, nor the one passed to `closest(selector, context)`, so if `pos` is `0` the context is just skipped. Depending on how you look at it, they either need to figure out a way to pass context to `matchesSelector`, or set `pos` correctly on just about any context other than `window`, so I agree, the issue could be fixed at that point as well. – adeneo Jan 03 '16 at 20:15
  • 1
    @adeneo Yup, I think we're on the same page. Thanks. I thought it was broken but needed someone to bounce it off of. Your answer (and discussion) were very helpful. I'll submit a bug report. They either need to change the documentation and remove this.context from pos, or make the non-pos version use this.context too. – James Montagne Jan 03 '16 at 20:19
  • Yes, this surely can't be what they intended. I do see that they have extended the regex for `pos` somewhere between 1.7 and 1.8 to match more cases, but it still has the same issues ? – adeneo Jan 03 '16 at 20:30
  • @adeneo Was attempting to modify Attribute Not Equal Selector [name!=”value”] selector recently , had to remove `"^"` at beginning of `RegExp` for `"ATTR"` at line 712. `needsContext` of `rneedsContext` appear to be at line 720 . Not certain how to access , change `matchExpr` , tried thus far using `$.expr` , though `matchExpr` regexps not extended from `Sizzle` ? – guest271314 Jan 03 '16 at 22:07
  • @guest271314 - `rneedsContext` is just an alias for `jQuery.expr.match.needsContext`, that's where the regex really is, but just changing the regex at runtime probably won't work as it's not passed by reference *(of a value)*, and it won't change all the places it's used etc. – adeneo Jan 03 '16 at 22:22
  • @adeneo fwiw, plnkr of deleted Question "How to extend, modify, or override jQuery Attribute Not Equal Selector [name!=”value”] selector?" http://plnkr.co/edit/uavSs1jcaMarbDKG1xfe . Was able to return expected result for `ul[class!='active']`, save for selector where space was within selector `body [class!='active']`. Also tried to create a `[data]` selector , similar to `[class]` or `[id]` , which would return all elements having `data-*` set at `html` or `jQuery.data()` . Documentation on how to adjust or extend selectors using `$.expr` appear sorely missing from jQuery documentation – guest271314 Jan 03 '16 at 22:25
  • Adjusted lines 713, 1699 at `script.js` at plnkr – guest271314 Jan 03 '16 at 22:32
  • @adeneo `.needsContext` does not appear to be extended to property of `$.expr.match` ? Try `$.expr.match` at `console` – guest271314 Jan 03 '16 at 22:41