tweak tie-breaking logic for Accept-Language header

previously in the case of ties in the quality value we sorted by length (so en
beats en-US), and then alphabetically (so ar beats en).  this part of the spec
is somewhat ambiguous, but this ordering does not take into account the order
passed into the accept header.  so passing "en,ar" would have chosen 'ar'
because quality and length both tie, and 'ar' is first alphabetically.

to fix this we are incorporating the order of the types passed into the accept
header as one of the tie breaking criteria.

this fixes the bug with CutyCapt snapshots of canvas being rendered in arabic
because CutyCapt passes "en,*" as it's accept header.  up until recently, 'en'
happened to be alphabetically first in our list of supported languages, so
everything worked out.  but when we added support for arabic, a new
alphabetical winner emerged, exposing this bug.

fixes CNVS-3070

test plan:
- in an environment with CutyCapt enabled
- create a web submission assignment that allows url submissions
- submit a canvas url that has this fix applied (note that you can run this
  from anywhere, it's the url you submit that needs the fix)
- it should be in english, not arabic

Change-Id: I7cf6d9db02ec0e79ad425bc0479c4fc43942fa52
Reviewed-on: https://gerrit.instructure.com/24348
Reviewed-by: Jon Jensen <jon@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Liz Abinante <labinante@instructure.com>
QA-Review: Bryan Madsen <bryan@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
This commit is contained in:
Simon Williams 2013-09-13 13:49:09 -06:00
parent 1926724eae
commit 0f25871915
2 changed files with 11 additions and 5 deletions

View File

@ -54,15 +54,19 @@ module LocaleSelection
best_locales = supported_locales.inject([]) { |ary, locale|
if best_range = ranges.detect { |r, q| r + '-' == (locale.downcase + '-')[0..r.size] || r == '*' }
ary << [locale, best_range.last] unless best_range.last == 0
ary << [locale, best_range.last, ranges.index(best_range)] unless best_range.last == 0
end
ary
}.sort_by{ |l, q| [-q, l.count('-'), l]}
}.sort_by{ |l, q, pos| [-q, pos, l.count('-'), l]}
# wrt the sorting here, rfc2616 doesn't specify which tag is preferable
# if there is a quality tie (due to prefix matching or otherwise).
# technically they are equally acceptable, though we'll just always go
# with the shorter one (and then alphabetical). this seems reasonable for
# scenarios like the following:
# technically they are equally acceptable. we've decided to break ties
# with:
# * position listed in header (tie here comes from '*')
# * length of locale (shorter first)
# * alphabetical
#
# this seems reasonable for scenarios like the following:
# given that i accept 'en'
# and canvas is localized in 'en-US', 'en-GB-oy' and 'en-CA-eh'
# then i should get 'en-US'

View File

@ -82,6 +82,8 @@ describe LocaleSelection do
ls.infer_browser_locale("pt-BR, *;q=0.9, pt;q=0", ['pt']).should be_nil
# no pt variants supported, so we get the first alternative
ls.infer_browser_locale("pt-BR, pt;q=0.9, *;q=0.8", ['es', 'fr']).should eql('es')
# equal matches sort by position before alphabetical
ls.infer_browser_locale("en, *", ['ar', 'en']).should eql('en')
end
end