AvailableCanonicalTimeZones is using a case-sensitive sort on case-insensitive data

[justingrant]

AvailableCanonicalTimeZones uses a case-sensitive sort on a list of IANA Time Zone Database identifiers. These identifiers are case-insensitive, so shouldn't the sort also be case-insensitive?

Thankfully there's only one IANA canonical time zone that is impacted by this: PST8PDT. This time zone is currently sorted before Pacific/* zones, but if it had been insensitively sorted then it'd be near the end of the list. Note that only Firefox includes the PST8PDT zone, while other implementations exclude this zone, so changing this behavior would have no impact on non-FF implementations.

function caseInsensitiveSortFunc(s1, s2) {
  s1 = s1.toLowerCase();
  s2 = s2.toLowerCase();
  if (s1 < s2) return -1;
  if (s1 > s2) return 1;
  return 0;
}

tzs = Intl.supportedValuesOf('timeZone');
tzsInsensitive = [...Intl.supportedValuesOf('timeZone')].sort(caseInsensitiveSortFunc);

console.log(`Index of PST8PDT (case sensitive): ${tzs.indexOf('PST8PDT')}`);
// => Index of PST8PDT (case sensitive): 426 
console.log(`Index of PST8PDT (case insensitive): ${tzsInsensitive.indexOf('PST8PDT')}`);
// => Index of PST8PDT (case insensitive): 466

[justingrant]

@anba - because this normative change would only affect Firefox, do you have an opinion about whether this change would be a good idea or not?

[anba]

I don't think it's necessary to change this. IIRC we only wanted this to be sorted to ensure the output is more deterministic. (Deterministic insofar as that consecutive calls to Intl.supportedValuesOf return the same output in the same order. And that we don't leak some implementation details to the user.)

[ben-allen]

@justingrant - what do you think? Move forward with this one, or close it?

[justingrant]

I still think it makes sense to be consistently case-insensitive for both sorting and comparison of identifiers. Seems weird to be sensitive for one but not for the other. I admittedly don't feel that strongly though. @gibson042 @sffc what do y'all think?

[gibson042]

I also lack strong feelings here, and since we currently have alignment between spec and implementation, my inclination is to leave the situation as-is.

[sffc]

The spec currently says:

4. Sort result in order as if an Array of the same values had been sorted using %Array.prototype.sort% using undefined as comparefn.

We stated previously that we could choose a different ordering based on use case but that we would use the undefined sort function when lacking a clear alternative. I can dig up the notes for that if you like.

I've personally had the position for a while that the bar for not using undefined sort should be somewhat high. I think there is a use case of binary-searching the big list, and it is less error-prone to do that if they are sorted in the ECMAScript standard order.

So my position is that we should keep the current spec and document in the style guide our norms for sorting return values.

[justingrant]

OK, works for me. Sounds like we'll leave the current behavior as-is.

Should I leave this open to remind folks to make the style-guide changes?

I also filed an MDN PR to warn users that time zones might be sorted unexpectedly: mdn/content#27890.

[ben-allen]

Style guide updated by #831

[justingrant]

The underlying problem should no longer occur because PST8PDT is now removed from the list of canonical time zones in IANA and CLDR.

I still think it would be sensible to use a case-insensitive sort instead of lexicographic for case-insensitive string data, but perhaps it's OK to wait to have that argument until there's a time that it will actually matter for observable results.

[gibson042]

I share that preference, and once removal of PST8PDT has fully propagated, we'll be in a position where the change from case-sensitive sorting to case-insensitive is unobservable, at which point I would feel comfortable making it. My primary motivation above for leaving things alone was avoiding visible churn, which will soon be a non-issue thanks to your efforts.

[ben-allen]

reopening based on @gibson042 and @justingrant comments, tagging as blocked until such time as PST8PDT removal propagates

[justingrant]

Cool. If we make this change, then we should also amend the style guidelines too.