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

Allows searching for & and other special characters #2157

Closed
wants to merge 2 commits into from
Closed

Allows searching for & and other special characters #2157

wants to merge 2 commits into from

Conversation

lode
Copy link

@lode lode commented Dec 5, 2014

This allows me to search for special characters like ampersand in select boxes. I have a client which has trouble without this as they have large selects and a few options are only findable via ampersands. (i.e. a&b will not be found easily as a and b are far too common).

I don't know if this might break something (why the html() was needed), but for me it seems to work.

@romank8k
Copy link

romank8k commented Jan 6, 2015

I have the same issue, but we're using the prototype version of Chosen.
I opened up a pull request for it here: #2186

@tjschuck
Copy link
Member

tjschuck commented Jan 6, 2015

I don't know if this might break something (why the html() was needed)

This function was added in #1339 by @pfiller

@tjschuck
Copy link
Member

tjschuck commented Jan 6, 2015

@lode Can you cherry-pick @rkhmelichek's commit over to your PR so this is just one PR?

@lode
Copy link
Author

lode commented Jan 6, 2015

@tjschuck, I'll do.

By the way, I don't think it was added by #1339, it was just moved, see https://github.com/harvesthq/chosen/pull/1339/files#diff-c82fb80b20c6f4dcdec5a08a57059c30L409. As far as I know it was added in aed8ba8.

@tjschuck
Copy link
Member

tjschuck commented Jan 6, 2015

@lode Good call -- that was in #89, which refs #73

@lode
Copy link
Author

lode commented Jan 6, 2015

@tjschuck, how do you prefer the merges to be done?
Shall I merge in master from harvesthq? Or do you rather have a rebase?

By the way, if you feel that this creates xss trouble again, please let me know.

@tjschuck
Copy link
Member

tjschuck commented Jan 6, 2015

Rebasing should be fine.

I can't give the +1 on the code change -- I'm mostly just a babysitter for Chosen, managing/triaging issues. One of the @harvesthq/chosen-developers will have to assess whether this will cause XSS or other issues.

@lode
Copy link
Author

lode commented Jan 6, 2015

I see @tjschuck. I added @rkhmelichek's commit (thanks @rkhmelichek for #2186!).

@pfiller
Copy link
Contributor

pfiller commented Mar 4, 2015

I don't think this is an issue as of 1.4.0 as evidenced in this fiddle. Perhaps the merging of #2254 made it all better?

In any event, I'm going to close this in the meantime. Thanks, @lode!

@pfiller pfiller closed this Mar 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants