Don't append Unicode extensions to a default locale that already has one - #5126
Open
jessealama wants to merge 2 commits into
Open
Don't append Unicode extensions to a default locale that already has one#5126jessealama wants to merge 2 commits into
jessealama wants to merge 2 commits into
Conversation
|
9 new or modified tests were run on 7 engines.
|
ptomato
reviewed
Sep 3, 2026
ptomato
left a comment
Member
There was a problem hiding this comment.
I'm not positive about what the right move is here. This seems to work fine. But on the other hand, it seems simpler (and arguably more robust) to insert the extension sequences in the right place with an addUExtension(key, value) helper function:
- If there's no
-u-, append-u-${key}-${value}. - If there is
-u-and no other single-letter subtags after it, append-${key}-${value}. - If there is
-u-and any other single-letter subtag after it, insert-${key}-${value}-right before the first such single-letter subtag.
…cale The Collator test for ignored invalid extension values now builds its baseline collator from the base name as well, so that its resolved-locale and resolved-options comparisons use the same locale as the collators under test.
jessealama
force-pushed
the
locale-base-name-helper
branch
from
September 6, 2026 21:02
ac404b9 to
bd837ab
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #615.
testNumberFormatand nine standalone tests build locales by appending strings like"-u-<key>-<value>"to the default locale coming fromresolvedOptions(). When a host reports a default locale that already carries a Unicode extension sequence (e.g."en-US-u-va-posix"), the result has two"-u-"substrings. This causes the constructor to throw aRangeErrorbefore these tests reach the behavior they're meant to check. JSC does this in CI, where 43 tests currently fail for that reason.This PR adds a new
getLocaleBaseNamehelper totestIntl.js. It strips extension and private-use subtags (thereby mirroringIntl.Locale.p.baseName). The helper is then applied at each site before any extension gets appended.Whether the default locale may contain an extension at all is a separate question, covered by a new conformance test in #5125.
The helper uses plain string juggling rather than
Intl.Locale. That's arguably a more proper solution, but usingIntl.Localewould add a feature dependency to legacy Collator and NumberFormat tests. I also notice that V8'sbaseNamecurrently returns"en-US-u-va-posix"unchanged for exactly this input, so the helper wouldn't do anything there. Switching toIntl.Localeonce both concerns are resolved would be a reasonable follow-up.