Skip to content

When no attribute exists return null instead of undefined #2129

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

When no attribute exists return null instead of undefined #2129

wants to merge 4 commits into from

Conversation

winhowes
Copy link
Contributor

@winhowes winhowes commented Mar 9, 2015

From issue #2118 returning null for non-existant attribute instead of undefined keeps in line with browser behavior for getAttribute().

@winhowes
Copy link
Contributor Author

winhowes commented Mar 9, 2015

Signed the CLA

@gibson042
Copy link
Member

We should be discussing #2118 at the next meeting, hopefully reaching a consensus on whether or not to accept it. In the meantime, this PR will need unit tests, and I also observe that ret is no longer necessary in that block... you can just directly return the value from jQuery.find.attr.

@winhowes
Copy link
Contributor Author

winhowes commented Mar 9, 2015

The reason I kept ret was in case some browser returned undefined instead of null. This would normalize those cases to null or is that not an issue?

@timmywil
Copy link
Member

timmywil commented Mar 9, 2015

It's not an issue. jQuery.find.attr will return null.

@timmywil
Copy link
Member

@winhowes Sorry for the stringency, but the CLA name and email must match the name and email in the commit. Could you sign again as "Winston" winstonhowes@gmail.com?

@mgol
Copy link
Member

mgol commented Mar 16, 2015

@winhowes Sorry for the stringency, but the CLA name and email must match the name and email in the commit. Could you sign again as "Winston" winstonhowes@gmail.com?

The CLA should be signed with a full name. Instead of signing the CLA with just the first name, the OP should modify the commit author so that the full name is used. We can do it in this case manually, but please @winhowes change your Git settings for the sake of the future.

@timmywil
Copy link
Member

Ah, wasn't sure about that rule. Ok, in that case, I'll merge with the full name.

@timmywil timmywil closed this in aaeed53 Mar 16, 2015
@winhowes
Copy link
Contributor Author

git settings changed, thanks for letting me know

timmywil pushed a commit that referenced this pull request Mar 16, 2015
@winhowes @timmywil
Attributes: return null when attribute does not exist
afca031 
Fixes gh-2118
Close gh-2129

Conflicts:
	test/unit/attributes.js
markelog pushed a commit that referenced this pull request Nov 10, 2015
@winhowes @markelog
Attributes: return null when attribute does not exist
bbfcf9b 
Fixes gh-2118
Close gh-2129
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@winhowes @gibson042 @timmywil @mgol @jquerybot