Skip to content

Commit

Permalink
Fix to avoid locale-dependent string comparisons
Browse files Browse the repository at this point in the history
It doesn’t seem appropriate to use locale dependent ordering to sort the
variants and extensions for normalization. 
The result of normalization should be consistent.

This commit removes a dependency on the ECMAScript internationalization API.
This API is not available everywhere, particularly on embedded devices like
TVs where storage space for internationalization data is
limited.

The new test case isn’t strictly valid - there should not be more than one
extension with the same singleton.

Closes GH-3.
  • Loading branch information
aoakley-roku authored Jul 17, 2023
1 parent 0143f52 commit ea42b0c
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 4 deletions.
16 changes: 12 additions & 4 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ import {likely} from './likely.js'

const own = {}.hasOwnProperty

const collator = new Intl.Collator()

/**
* @param {Schema} base
* @param {Partial<Schema>} changes
Expand Down Expand Up @@ -159,7 +157,7 @@ export function bcp47Normalize(tag, options) {
removeLikelySubtags(schema)

// 5. Sort variants, and sort extensions on singleton.
schema.variants.sort(collator.compare)
schema.variants.sort()
schema.extensions.sort(compareSingleton)

// 6. Warn if fields (currently only regions) should be updated but have
Expand Down Expand Up @@ -323,5 +321,15 @@ function add(object, key, value) {
* @returns {number}
*/
function compareSingleton(left, right) {
return collator.compare(left.singleton, right.singleton)
if (left.singleton > right.singleton) {
return 1
}

if (left.singleton < right.singleton) {
return -1
}

// It is invalid to have more than one extension with the same singleton so
// we should never reach this code.
return 0
}
1 change: 1 addition & 0 deletions test.js
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ test('fixtures', function () {
'en-us': 'en',
'en-gb': 'en-GB',
'en-us-x-twain': 'en-x-twain',
'en-a-ext1-a-ext2': 'en-a-ext1-a-ext2',
'en-a-myext-b-another': 'en-a-myext-b-another',
'en-bu': 'en-MM',
en: 'en',
Expand Down

0 comments on commit ea42b0c

Please sign in to comment.