Skip to content

IntlCollator: honor locale-tailored caseFirst default (da, mt) - #365

Open
robobun wants to merge 1 commit into
mainfrom
farm/89e6ff1f/intl-collator-casefirst-locale-default
Open

IntlCollator: honor locale-tailored caseFirst default (da, mt)#365
robobun wants to merge 1 commit into
mainfrom
farm/89e6ff1f/intl-collator-casefirst-locale-default

IntlCollator: honor locale-tailored caseFirst default (da, mt)

964a64f
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Jul 28, 2026 in 17m 42s

Code review found 1 potential issue

Found 1 candidates, confirmed 1. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 0
🟣 Pre-existing 1
Severity File:Line Issue
🟣 Pre-existing Source/JavaScriptCore/runtime/IntlCollator.cpp:264-265 Locale-tailored caseFirst still overwritten for unrecognized -u-kf- values

Annotations

Check notice on line 265 in Source/JavaScriptCore/runtime/IntlCollator.cpp

See this annotation in the file changed.

@claude claude / Claude Code Review

Locale-tailored caseFirst still overwritten for unrecognized -u-kf- values

Pre-existing edge case not covered by this fix: an unrecognized `-u-kf-` value (e.g. `new Intl.Collator('da-u-kf-xyz')` or bare `'da-u-kf'`) still forces `UCOL_OFF` and reports `caseFirst: "false"`, because `resolveLocale` falls back to `keyLocaleData[0]` — which `sortLocaleData` hardcodes as `"false"` regardless of locale — so `caseFirstString` is non-null and the read-back is skipped. The PR doesn't regress this (it was already `"false"` before) and correctly fixes the far-more-common no-exten