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

Properly escape CSS notation #1119

Closed
wants to merge 1 commit into from
Closed

Conversation

fkaempfer
Copy link

Small problem caused by #755 as it does not escape CSS notation within the id attribute properly. Compare: https://learn.jquery.com/using-jquery-core/faq/how-do-i-select-an-element-by-an-id-that-has-characters-used-in-css-notation/

@coveralls
Copy link

coveralls commented Jul 28, 2016

Coverage Status

Changes Unknown when pulling d29777c on fkaempfer:master into * on selectize:master*.

@joallard
Copy link
Member

Can you point to an example id where that would be the case?

@fkaempfer
Copy link
Author

Sure. One of the most popular Java web frameworks (JSF) creates IDs separated by ":" depending on their position in the DOM, for example "myForm:mySelectField". The ":" causes an exception in the label selector code.

@joallard
Copy link
Member

Given I have a label and input field with id 'form:name'
When I selectize this field
Then the resulting field should have id 'form:name-selectized'
And the label should have attr for 'form:name-selectized'

As I understand, that's not currently the case?

@joallard
Copy link
Member

Got it. That said, there's an even easier fix: http://stackoverflow.com/q/9763323/720164

- $('label[for='+inputId+']').attr('for', inputId + '-selectized');
+ $("label[for='"+inputId+"']").attr('for', inputId + '-selectized');

I can let you commit that if you want the credit ;o)

@joallard joallard added this to the 0.12.3 milestone Jul 28, 2016
@fkaempfer
Copy link
Author

Great, yeah that is much simpler. I checked that it works in my case.

No need to give me credit, but if it makes it easier for you to integrate the fix I can update the PR.

Thanks!

joallard added a commit that referenced this pull request Jul 29, 2016
@joallard
Copy link
Member

Fixed, thanks!

@joallard joallard closed this Jul 29, 2016
bwilson-ux pushed a commit to bwilson-ux/selectize.js that referenced this pull request Oct 4, 2016
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.

3 participants