Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

$(selector, context) uses wrong context #193

Closed
Philipp15b opened this issue Apr 17, 2013 · 11 comments
Closed

$(selector, context) uses wrong context #193

Philipp15b opened this issue Apr 17, 2013 · 11 comments

Comments

@Philipp15b
Copy link

I'm not sure whether this is intended behavior: When using $(selector, context), cheerio uses the parent context instead of the provided one. This leads to unintentionally searching the object's siblings too.

I find this confusing, because jQuery uses the context directly - this means it behaves exactly like calling find:

Internally, selector context is implemented with the .find() method, so $( "span", this ) is equivalent to $( this ).find( "span" ).

(from the jQuery Docs)

@Philipp15b
Copy link
Author

Is this maybe related to #70?

@davidchambers
Copy link
Contributor

I'm not sure whether this is intended behavior

This certainly sounds unintentional.

@ssmout
Copy link

ssmout commented Apr 20, 2013

This sounds like the problem I've been having in cheerio 0.10.5 and up. It worked in 0.10.4. I also commented about this on #167.

@fb55
Copy link
Member

fb55 commented Apr 20, 2013

I tried to implement proper context handling in CSSselect yesterday, but apparently my approach didn't handle all cases. When I find some time, I'll have a second look at it.

@trevorsheridan
Copy link

I'm experiencing this issue too. I'm trying to do something similar to the following and it seems like it's using the parent or root as the context.

$("section").each(function(index, element) {
    var url = $("a", this).attr("href");
});

@trevorsheridan
Copy link

@ssmout is right, it does work in 0.10.4. I suppose I will use this for now.

@skerit
Copy link

skerit commented May 8, 2013

This issue is still present in 0.11. Using version 0.10.4 still works.

@davidchambers
Copy link
Contributor

Fixed in a47bc68. I think this warrants bumping to 0.12.0, @matthewmueller.

@Philipp15b
Copy link
Author

👍 This is a breaking change, so a note in the README would be nice too.

@davidchambers
Copy link
Contributor

Or in History.md.

@matthewmueller
Copy link
Member

Updated to 0.12.0. Thanks guys

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

7 participants