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

[robobun]

CLDR's collation data for da (Danish) and mt (Maltese) specifies [caseFirst upper]: a default collator for those locales should sort uppercase before lowercase and report resolvedOptions().caseFirst === "upper" (V8/ICU honor this). JSC instead reported "false" and sorted lowercase first.

Cause

initializeCollator always called ucol_setAttribute(UCOL_CASE_FIRST, UCOL_OFF) when no caseFirst option or -u-kf- extension was supplied, overwriting the ICU locale tailoring, and derived m_caseFirst only from the option/extension value (never from the opened collator).

Fix

resolveLocale already leaves the Kf extension as a null String when neither an option nor a Unicode extension requests a value (see the early continue in IntlObject.cpp). When it is null:

• skip the ucol_setAttribute(UCOL_CASE_FIRST, ...) call so the locale's tailored default stays in effect, and
• read ucol_getAttribute(UCOL_CASE_FIRST) back into m_caseFirst so resolvedOptions() reports the actual value and updateCanDoASCIIUCADUCETComparison() gates the ASCII fast path correctly.

This mirrors the existing handling of UCOL_ALTERNATE_HANDLING / m_ignorePunctuation directly above it.

Explicit { caseFirst: "false" } or -u-kf-false still produce a non-null Kf extension and continue to force UCOL_OFF as before.

Repro

for (const loc of ["da", "mt", "en"]) {
  const c = new Intl.Collator(loc);
  console.log(loc, c.resolvedOptions().caseFirst, c.compare("a", "A"));
}
// before: da "false" -1 | mt "false" -1 | en "false" -1
// after:  da "upper"  1 | mt "upper"  1 | en "false" -1

Branched from 549170099226 (Bun's current WEBKIT_VERSION) so the paired Bun PR can pin the preview build without pulling in unrelated WebKit changes.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 4 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 814d499f-15a8-4b3f-982c-ec60b61de3a8

📥 Commits

Reviewing files that changed from the base of the PR and between 9839239 and 964a64f.

📒 Files selected for processing (1)

• Source/JavaScriptCore/runtime/IntlCollator.cpp

---

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

Beyond the inline note, I also verified: (1) resolveLocale in IntlObject.cpp:1048 does leave extensions[Kf] as a null String when neither an option nor a -u-kf- subtag is present, so the isNull() gate is sound; (2) the ASCII fast path stays safe — canDoASCIIUCADUCETComparisonWithUCollator independently checks UCOL_CASE_FIRST == UCOL_OFF on the collator, and m_caseFirst now agrees with it for da/mt.

Extended reasoning...

Confirmed the two load-bearing assumptions the fix rests on. First, the early continue at IntlObject.cpp:1048 (keyPos == notFound && !optionsValue) skips writing resolved.extensions[Kf], so caseFirstString.isNull() reliably distinguishes the no-option/no-extension path from an explicit "false". Second, the ASCII UCADUCET fast path was already double-gated: even before this change, canDoASCIIUCADUCETComparisonWithUCollator read UCOL_CASE_FIRST directly from the collator and would have rejected da/mt once the setAttribute overwrite is removed — the read-back into m_caseFirst just makes the earlier m_caseFirst != CaseFirst::False short-circuit consistent with it. No new fast-path risk introduced.

[claude]

🟣 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-extension path; a possible follow-up is to make sortLocaleData's Kf list locale-aware, or to also read back when the resolved value is "false" but no explicit option/extension requested it.

Extended reasoning...

What the bug is

The PR's read-back of UCOL_CASE_FIRST fires only when caseFirstString.isNull(). That correctly catches the no-extension / no-option case, but there is one more path where the caller did not actually specify a caseFirst yet caseFirstString comes back non-null: a -u-kf- Unicode extension whose value is not one of upper / lower / false (e.g. 'da-u-kf-xyz', or a bare 'da-u-kf'). On that path the locale's tailored [caseFirst upper] is still overwritten with UCOL_OFF.

The specific code path

In resolveLocale (IntlObject.cpp:1041–1069):

1. For 'da-u-kf-xyz', extensionSubtags contains kf, so keyPos != notFound and the early continue at line 1048 is not taken.
2. keyLocaleData = sortLocaleData("da", Kf) returns ["false", "lower", "upper"] — hardcoded independent of locale (IntlCollator.cpp:104).
3. value = keyLocaleData[0] → "false" (line 1054).
4. The requested value "xyz" is not found in keyLocaleData (line 1060–1061), so value stays "false". For a bare 'da-u-kf', keyLocaleData doesn't contain "true" (line 1065), so value also stays "false".
5. resolved.extensions[Kf] is the non-null string "false".

Back in initializeCollator:

• caseFirstString == "false" → m_caseFirst = CaseFirst::False → local caseFirst = UCOL_OFF.
• !caseFirstString.isNull() is true, so ucol_setAttribute(UCOL_CASE_FIRST, UCOL_OFF) still runs, clobbering da's CLDR [caseFirst upper] tailoring.
• The new read-back block is skipped because caseFirstString.isNull() is false.

Why existing code doesn't prevent it

Per ECMA-402 §10.2.3, [[LocaleData]].[[<locale>]].kf's first element should be the locale's default ("upper" for da/mt), so an unrecognized extension value should fall back to that default. sortLocaleData/searchLocaleData instead hardcode ["false", "lower", "upper"] for every locale, so keyLocaleData[0] is always "false". This is a pre-existing spec-compliance gap; the PR's isNull() gate can't distinguish “user explicitly asked for false” from “resolveLocale fell back to element 0.”

Impact

After this PR:

• new Intl.Collator('da').resolvedOptions().caseFirst === 'upper' ✅ (fixed)
• new Intl.Collator('da-u-kf-xyz').resolvedOptions().caseFirst === 'false' and sorts lowercase-first, whereas V8/ICU return 'upper'.

This is unchanged from before the PR — 'da-u-kf-xyz' already returned "false" and forced UCOL_OFF. The PR is a strict improvement and does not regress this edge case. The trigger (users passing an invalid -u-kf- value to a da/mt collator) is quite obscure.

Step-by-step proof

Input: new Intl.Collator('da-u-kf-xyz')
  requestedLocales     = ['da-u-kf-xyz']
  caseFirstOption      = null (no options object)
  → resolveLocale:
      extensionSubtags = ['kf', 'xyz']
      keyPos('kf')     = 0  ≠ notFound  → skip line-1048 continue
      keyLocaleData    = sortLocaleData('da', Kf) = ['false','lower','upper']
      value            = keyLocaleData[0] = 'false'
      'xyz' ∉ keyLocaleData → value stays 'false'
      resolved.extensions[Kf] = 'false'   (non-null)
  → initializeCollator:
      caseFirstString  = 'false'  → m_caseFirst = False → caseFirst = UCOL_OFF
      !caseFirstString.isNull()   → ucol_setAttribute(UCOL_CASE_FIRST, UCOL_OFF)  ← overwrites da tailoring
      caseFirstString.isNull() == false → read-back block skipped
Result: resolvedOptions().caseFirst == 'false', compare('a','A') == -1

Possible fixes (follow-up)

Either would close the gap:

• Make sortLocaleData/searchLocaleData locale-aware for Kf (return ["upper", ...] for da/mt) so keyLocaleData[0] is the actual locale default per spec; or
• Also take the read-back path when caseFirstString == "false"_s && caseFirstOption.isNull() (i.e., "false" came from fallback rather than an explicit {caseFirst: 'false'} / -u-kf-false). Explicit -u-kf-false would still round-trip correctly because it is in keyLocaleData and produces the same non-null "false", so an extra flag distinguishing “explicit vs fallback” may be needed for full precision.

[github-actions]

Preview Builds

Commit	Release	Date
964a64fa	autobuild-preview-pr-365-964a64fa	2026-07-28 08:52:35 UTC