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

Use $.data() less #424

Merged
merged 1 commit into from
Dec 9, 2020
Merged

Use $.data() less #424

merged 1 commit into from
Dec 9, 2020

Conversation

eritbh
Copy link
Member

@eritbh eritbh commented Dec 9, 2020

$.data() tries to read from jQuery's data cache or from data-* attributes. When it does the latter, it tries to process attribute values as non-string values if it thinks that's appropriate. It's rarely right.

Fixes #422 - using $.data('subreddit') on elements with a data-subreddit value with a subreddit name that's numeric was doing the wrong thing. $.attr('data-subreddit') always returns a string (and skips the jQuery data cache, which is also good for clarity.)

We should try to make an effort to remove other uses of $.data() as well, but this is all I'm gonna do for this PR.

@eritbh eritbh added bug something isn't working code quality labels Dec 9, 2020
@eritbh eritbh added this to the v5.5 milestone Dec 9, 2020
@creesch
Copy link
Member

creesch commented Dec 9, 2020

LGTM, I honestly thought we already made an effort once to remove usage of it a few years ago exactly because of this sort of stuff...

@creesch creesch requested review from creesch and removed request for creesch December 9, 2020 08:11
@creesch creesch merged commit a765880 into master Dec 9, 2020
@creesch creesch deleted the jq-data-bad branch December 9, 2020 08:11
eritbh added a commit that referenced this pull request Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something isn't working code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Toolbox breaks if subreddit name is all numbers
2 participants