Skip to content

Commit

Permalink
Don't search escaped HTML text
Browse files Browse the repository at this point in the history
Before this change, if you tried to search for "&" to match "Stuff
& Nonsense", what we would do is html-escape _both_ things and compare
the escaped strings.

This works fine in the case where you just type "&". However, if you
type "mp", that will match the html entity "&" in the middle of the
html-escaped text we're comparing it to. Which is incorrect.

Additionally, we highlight our "matched" result. When we do, we break up
the entity so that the resulting HTML looks like:

    Stuff &a<em>mp</em>; Nonsense

And you get to see what basically looks like garbage on the screen.

![screen garbage](https://camo.githubusercontent.com/b5e384f3f622f3560739c287c481e05a7b0668af/68747470733a2f2f646c2e64726f70626f7875736572636f6e74656e742e636f6d2f7370612f7470786279757478626b747069347a2f6c7566336c7835792e706e67)

This commit fixes all of that by just comparing the actual text of
things. We avoid the problem previous addressed in #2254 by escaping
each piece of the to-be-highlighted option text before assembling it
into HTML.
  • Loading branch information
Arun Srinivasan committed Aug 22, 2017
1 parent 18a4c0b commit 2c96f90
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 5 deletions.
2 changes: 1 addition & 1 deletion coffee/chosen.jquery.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ class Chosen extends AbstractChosen
@search_field.val()

get_search_text: ->
this.escape_html $.trim(this.get_search_field_value())
$.trim this.get_search_field_value()

escape_html: (text) ->
$('<div/>').text(text).html()
Expand Down
2 changes: 1 addition & 1 deletion coffee/chosen.proto.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ class @Chosen extends AbstractChosen
@search_field.value

get_search_text: ->
this.escape_html this.get_search_field_value().strip()
this.get_search_field_value().strip()

escape_html: (text) ->
text.escapeHTML()
Expand Down
9 changes: 6 additions & 3 deletions coffee/lib/abstract-chosen.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -185,19 +185,22 @@ class AbstractChosen
results += 1 if results_group.active_options is 0 and results_group.search_match
results_group.active_options += 1

option.search_text = if option.group then option.label else option.html
option.search_text = if option.group then option.label else option.text

unless option.group and not @group_search
search_match = this.search_string_match(option.search_text, regex)
option.search_match = search_match?

results += 1 if option.search_match and not option.group


if option.search_match
if searchText.length
startpos = search_match.index
text = option.search_text.substr(0, startpos + searchText.length) + '</em>' + option.search_text.substr(startpos + searchText.length)
option.search_text = text.substr(0, startpos) + '<em>' + text.substr(startpos)
prefix = option.search_text.slice(0, startpos)
fix = option.search_text.slice(startpos, startpos + query.length)
suffix = option.search_text.slice(startpos + query.length)
option.search_text = "#{this.escape_html(prefix)}<em>#{this.escape_html(fix)}</em>#{this.escape_html(suffix)}"

results_group.group_match = true if results_group?

Expand Down
28 changes: 28 additions & 0 deletions spec/jquery/searching.spec.coffee
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
describe "Searching", ->
it "should not match the actual text of HTML entities", ->
tmpl = "
<select data-placeholder='Choose an HTML Entity...'>
<option value=''></option>
<option value='This & That'>This &amp; That</option>
<option value='This < That'>This &lt; That</option>
</select>
"

div = $("<div>").html(tmpl)
select = div.find("select")
select.chosen({search_contains: true})

container = div.find(".chosen-container")
container.trigger("mousedown") # open the drop

# Both options should be active
results = div.find(".active-result")
expect(results.size()).toBe(2)

# Search for the html entity by name
search_field = div.find(".chosen-search input").first()
search_field.val("mp")
search_field.trigger("keyup")

results = div.find(".active-result")
expect(results.size()).toBe(0)
30 changes: 30 additions & 0 deletions spec/proto/searching.spec.coffee
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
describe "Searching", ->
it "should not match the actual text of HTML entities", ->
tmpl = "
<select data-placeholder='Choose an HTML Entity...'>
<option value=''></option>
<option value='This & That'>This &amp; That</option>
<option value='This < That'>This &lt; That</option>
</select>
"

div = new Element('div')
document.body.insert(div)
div.update(tmpl)
select = div.down('select')
new Chosen(select, {search_contains: true})

container = div.down('.chosen-container')
simulant.fire(container, 'mousedown') # open the drop

# Both options should be active
results = div.select('.active-result')
expect(results.length).toBe(2)

# Search for the html entity by name
search_field = div.down(".chosen-search input")
search_field.value = "mp"
simulant.fire(search_field, 'keyup')

results = div.select(".active-result")
expect(results.length).toBe(0)

0 comments on commit 2c96f90

Please sign in to comment.