Skip to content

Potential fix for .matches when using an attribute selector against attributes with no value#54

Closed
mmurray wants to merge 3 commits intojquery:masterfrom
mmurray:bug_7770
Closed

Potential fix for .matches when using an attribute selector against attributes with no value#54
mmurray wants to merge 3 commits intojquery:masterfrom
mmurray:bug_7770

Conversation

@mmurray
Copy link

@mmurray mmurray commented Mar 26, 2011

Sizzle.matches fails to match based on an attribute selector when the attribute has no value.

I discovered this bug in Sizzle while investigating jQuery bug #7770, this pull request contains my proposed fix and some regression tests.

Basically, if there is no operator type then I assume we are just checking for existence of the attribute. So as long as the value of result is not null or undefined then we should have a match.

@timmywil
Copy link
Member

@mmurray
Copy link
Author

mmurray commented Mar 26, 2011

Ah, I should have pulled first, heh. Thanks!

@mmurray mmurray closed this Mar 26, 2011
@mmurray
Copy link
Author

mmurray commented Mar 26, 2011

Are you sure it's the same bug? I mistakenly said matchesSelector at first, but it's actually an issue with SIzzle.matches. After making sure I'm up-to-date on master the first test in this pull request still fails, and it is fixed by my change.

@mmurray mmurray reopened this Mar 26, 2011
@timmywil
Copy link
Member

Hmm, I thought it was. Maybe I was wrong.

@timmywil
Copy link
Member

I was working on a jQuery bug related to 7770 and noticed that it was fixed. That was http://bugs.jquery.com/ticket/7369

@timmywil
Copy link
Member

timmywil commented May 7, 2011

I'm starting to think this pull might fix several jQuery selector bugs. We should take a close look at this soon.

@timmywil
Copy link
Member

timmywil commented May 7, 2011

@murz: could you also take a look at #5637, #6358, and #7128. I'm not sure they are related, but they seem similar to me.

@mmurray
Copy link
Author

mmurray commented Jul 11, 2011

Sorry I have been busy and haven't had a chance to look at those yet. I will try to take a look this week.

@timmywil
Copy link
Member

I don't this is necessary anymore.

@timmywil timmywil closed this Jan 23, 2012
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.

2 participants