Skip to content

Conversation

@anba
Copy link
Contributor

@anba anba commented May 26, 2025

These tests currently fail in V8, but pass in JSC and SM.

@anba anba requested a review from a team as a code owner May 26, 2025 09:19
Copy link
Contributor

@Ms2ger Ms2ger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

…are present

These tests currently fail in V8, but pass in JSC and SM.
@Ms2ger Ms2ger force-pushed the resolved-options branch from 1ce9da3 to 641129c Compare June 1, 2025 11:01
@Ms2ger Ms2ger merged commit 95d90bf into tc39:main Jun 1, 2025
11 checks passed
@anba anba deleted the resolved-options branch June 4, 2025 12:36
// resolved locale.
{
locale: "en-u-nu-arab",
numberingSystem: "invalid",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which line in the spec prevent "invalid" or "invalid2" to be used for numberingSystem?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The relevant spec operation is ResolveLocale, step 13.k:

k. If SameValue(optionsValue, value) is false and keyLocaleData contains optionsValue, then

In the test case, optionsValue is "invalid" and value is "arab". SameValue("invalid", "arab") returns false, but because keyLocaleData doesn't contain a numbering system named "invalid", optionsValue is ignored and the numbering system defined in the Unicode extension sequence u-nu-arab is used.

This assumes that no valid numbering system is named "invalid". If it helps, we can adjust the test case to also include:

assert.sameValue(
  Intl.supportedValuesOf("numberingSystem").includes("invalid"),
  false,
  "'invalid' is not a valid numbering system"
);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants