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

Escape jQuery identifiers #1796

Merged
merged 3 commits into from Nov 17, 2016
Merged

Escape jQuery identifiers #1796

merged 3 commits into from Nov 17, 2016

Conversation

serveace
Copy link
Contributor

Austen Holmes added 2 commits May 13, 2016 17:04
If you have an identifier with a . character in it, for example, <input id="name.first" /> jQuery will try to find an element with id "name" and class "first".  This is unexpected behavior.  In this case, the "." must be escaped with a backslash.
@Shadowfiend Shadowfiend added this to the 3.1 milestone May 18, 2016
@eltimn
Copy link
Member

eltimn commented May 18, 2016

If we're going to fix this, should we fix it for all of the meta characters where this is a problem?

This SO entry has some good info on the problem.

I think using this is probably the best way to go:

jQuery(document.getElementById(elementOrId))

@serveace
Copy link
Contributor Author

Good call. I'll update my local env and test a little bit. If it looks good there, I'll update the pr.

Instead of trying to escape id selectors, use document.getElementById
@fmpwizard
Copy link
Member

quick question, why is this marked for 3.1 and not for the next RC?

@Shadowfiend
Copy link
Member

IIRC, it's not a regression, so there was no reason to hold up a 3.0 final release for it (this was opened after th RC cycle began).

@fmpwizard
Copy link
Member

ok, and I imagine we don't want to add it now to the next RC in case of a regression caused by this fix, right? (because there will be another RC to handle that one js bug we still have)

@Shadowfiend
Copy link
Member

Right. Basically I think the RCs should be frozen to forward development; we should only fix regressions.

@Shadowfiend
Copy link
Member

Soo I think we can move forward on this. @eltimn it sounds like you had a look here? We good to go?

@eltimn
Copy link
Member

eltimn commented Nov 17, 2016

Yes. Looks good.

@Shadowfiend Shadowfiend merged commit 5cc1e2e into lift:master Nov 17, 2016
@Shadowfiend
Copy link
Member

All in then!

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

Successfully merging this pull request may close these issues.

None yet

4 participants