Skip to content
This repository has been archived by the owner on Jan 25, 2022. It is now read-only.

no validation of value of extensions in tag? #81

Closed
FrankYFTang opened this issue Sep 13, 2019 · 8 comments
Closed

no validation of value of extensions in tag? #81

FrankYFTang opened this issue Sep 13, 2019 · 8 comments
Milestone

Comments

@FrankYFTang
Copy link
Contributor

I am not sure my observation is correct below. Need some help.
It seems to me, with the current spec, we will valid the value of options. For example, if we put in new Intl.Locale("en", {caseFirst: "true"}) or Intl.Loclae("en", {caseFirst: "mom"}) we will throw RangeError because we only pass « "upper", "lower", "false" » to GetOption in https://tc39.es/proposal-intl-locale/#sec-Intl.Locale which does not contains "true" or "mom"

However, what will happen if we call
new Intl.Locale("en-u-kf-true") or new Intl.Locale("en-u-kf-mom") ?
I somehow cannot see anywhere in the spec cause us to throw RangeError.
Is that true, or is it because I missed something.

What should the expected result of
(new Intl.Locale("en-u-kf-true")).caseFirst and
(new Intl.Locale("en-u-kf-mom")).caseFirst ?
@littledan @sffc @Ms2ger @zbraniecki @anba

@zbraniecki
Copy link
Member

I think this is a reasonable behavior tbh.

We verify that the input language identifier string is well formed while we verify that the options are valid.

We're consistent in that behavior.

Long time ago we talked about an option to validate the input string as well, but that never came to fruition.

@FrankYFTang
Copy link
Contributor Author

then
What should the expected result of
(new Intl.Locale("en-u-kf-true")).caseFirst and
(new Intl.Locale("en-u-kf-mom")).caseFirst ?

@zbraniecki
Copy link
Member

(new Intl.Locale("en-u-kf-true")).caseFirst

undefined

(new Intl.Locale("en-u-kf-mom")).caseFirst

undefined

I'm not opposed to liberating options to be well-formed only, in which case we'd also accept caseFirst = "mom" and just not apply it since it's not valid.
That would be forward compatible, but I'm not sure if consistency is more important than feedback to users.

The reason why I think the current subset makes sense (until we complete it) is that options come from developers, so throwing on well-formed, but invalid value makes sense.
langid strings come from some external sources, browser, system etc. If they contain errors, rejecting them should be last resort, because user can rarely do anything about it, and the dev usually cannot do anything.

@FrankYFTang
Copy link
Contributor Author

(new Intl.Locale("en-u-kf-true")).caseFirst

undefined

(new Intl.Locale("en-u-kf-mom")).caseFirst

undefined

Why, according to the current spec, the result is not "true" and "mom" ? That is the part I try to understand.

I'm not opposed to liberating options to be well-formed only, in which case we'd also accept caseFirst = "mom" and just not apply it since it's not valid.

I am not lobbying for that. Just try to figure out what the current spec said.

That would be forward compatible, but I'm not sure if consistency is more important than feedback to users.

The reason why I think the current subset makes sense (until we complete it) is that options come from developers, so throwing on well-formed, but invalid value makes sense.
langid strings come from some external sources, browser, system etc. If they contain errors, rejecting them should be last resort, because user can rarely do anything about it, and the dev usually cannot do anything.

@littledan
Copy link
Member

littledan commented Sep 18, 2019

We last discussed this topic in #19 . There, we decided to validate simply the grammar, and not the contents, in these contexts. Applying that decision here, I think we should return "mom" and "true", as the current specification does, while continuing to be more strict in the options bag. (EDIT: @anba has the correct answers below)

@anba
Copy link
Contributor

anba commented Sep 23, 2019

The correct results are:

js> addIntlExtras(Intl) // Add Intl.Locale to Intl in SpiderMonkey shell
js> (new Intl.Locale("en-u-kf-true")).caseFirst
""
js> (new Intl.Locale("en-u-kf-mom")).caseFirst  
"mom"

The caseFirst result for "en-u-kf-true" is the empty string, because the tag is first canonicalised to "en-u-kf" per BCP 47 Language Tag to Unicode BCP 47 Locale Identifier and 3.2.1 Canonical Unicode Locale Identifiers which states:

Any type or tfield value "true" is removed.

@zbraniecki zbraniecki added this to the v1 milestone Oct 24, 2019
@zbraniecki
Copy link
Member

Based on my read of the current spec and @anba's description, it seems like the current spec is correct and the behavior is as expected.

Is it ok for me to close this issue?

@zbraniecki
Copy link
Member

reopen if needed!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants