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

Fix unexpected behavior while querying child #796

Merged
merged 2 commits into from
Jan 28, 2016
Merged

Fix unexpected behavior while querying child #796

merged 2 commits into from
Jan 28, 2016

Conversation

Delgan
Copy link
Contributor

@Delgan Delgan commented Jan 2, 2016

I followed the solution of fb55 and I added a unit test to ensure that #742 and #780 was correctly fixed.

Delgan and others added 2 commits January 2, 2016 12:10
This test case is intended to highlight two unwanted behaviors:
- The first one is related to issue #742. While using css-select v1.0.0, the child selector '>' would also include indirect descendants and this test case would return '3'.
- The second issue (discussed here: #780 (comment) ) comes while using css-select v1.2.0. In such a case, the context of the '>' selector is the first 'bar' and not 'foo' as intended, which causes a resulting length of '2'.
The issue, after upgrading css-select to v1.2.0, was due to an improper
context of options passed to 'CSSselect()'.
@jugglinmike
Copy link
Member

This looks good to me. I'm not so hot on using __proto__, but there's precedence in the project for it, so I'll leave that discussion for another time.

@fb55 Did you author commit 4779762?

@fb55
Copy link
Member

fb55 commented Jan 2, 2016

@jugglinmike No, I did not. It follows a suggestion I made, @Delgan probably wanted to give credit (which I appreciate!). A mention in the commit message would be better IMHO, but I don't care enough about it to force a change.

LGTM otherwise!

@jackwilsdon
Copy link

+1 on this, having the same issue.

@adelgado adelgado mentioned this pull request Jan 27, 2016
@adelgado
Copy link

This indeed fixes the issue, thanks :-)

I've seen elsewhere in the repo that releases are request-based, @fb55 is there anything missing for this PR to it be merged and a version containing the fix to be released? Thanks

fb55 added a commit that referenced this pull request Jan 28, 2016
Fix unexpected behavior while querying child
@fb55 fb55 merged commit 8c9b2e0 into cheeriojs:master Jan 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants