Locale Query Parameter taking long values like "espan" and loading like it is "es"

[kathy-phet]

Today, we discovered that on PhET Brand, the locale query parameter is doing something somewhat un-expected:
If you type in https://phet.colorado.edu/sims/html/center-and-variability/latest/center-and-variability_all.html?locale=esnas
the sim will load in "es" spanish.

It seems like with this value, it should fall back to "en" because esnas is not a valid value for the locale query parameter.

@jonathanolson - It seems like this is code that you might be working on with the locale improvements you are doing right now. Is that true?

[samreid]

We also observed, e.g., that https://phet.colorado.edu/sims/html/center-and-variability/latest/center-and-variability_all.html?locale=swedish comes up Swahili.

[jonathanolson]

Would we want it to pop up the "invalid query parameter" dialog, if that exists?

[pixelzoom]

These bits of code look relevant. This bug seems related to a desired to (for example) default 'zh_CN' => 'zh'. But it's buggy because there's no checking for the locale format and where the '_' appears.

localeProperty.ts:

// We might use a partial locale (e.g. 'en' instead of 'en_US'), so grab this if it exists. It might be the same as
// phet.chipper.locale (that's OK).
const partialLocale = typeof phet.chipper.locale === 'string' ? phet.chipper.locale.slice( 0, 2 ) : undefined;

localeOrderProperty.ts

  // Attempt to fill in a language reduction for the selected locale, e.g. 'zh_CN' => 'zh'
  const shortLocale = locale.slice( 0, 2 ) as Locale;
  if ( locale !== shortLocale && !localeOrder.includes( shortLocale ) ) {
    localeOrder.push( shortLocale );
  }

[kathy-phet]

Would we want it to pop up the "invalid query parameter" dialog, if that exists?

@jonathanolson - I believe, yes, we would want it to pop up that invalid query parameter note if its not a valid value at all:
So
on a full sim-scale locale, if the zh_CN translation doesn't exist at all, pop up the not a valid value,
on the within a simulation scale at the individual strings level, if you are in the zh_CH translated version of a sim and its missing a string, then fall back to zh

But please chime in agreement or with another thought @pixelzoom @zepumph @arouinfar from today's discussion.

So the "PhET brand" would pop up the not a valid option note.
But if the query parameter was added onto a PhET-iO Standard Wrapper, it wouldn't fail out (is that correct)? It would just stick with the default that the Standard Wrapper was saved with. Correct, @zepumph?

[zepumph]

We are in control with what we consider to be the schema of a locale string, so potentially the solution is as easy as adding _ as a check as the third char before checking on the first two chars. I think the desired behavior is to just go to the fallback locale with an invalid locale (which seems to be what people above are saying too).

on a full sim-scale locale, if the zh_CN translation doesn't exist at all, pop up the not a valid value,

I thought that we don't want this since we have started using ?locale to drive sim pages on the website. I can't exactly remember the conversation though. It worries me a bit to add that.

Looks like some paper trail is in #905, where the note about NOT adding public to ?locale was committed.

But if the query parameter was added onto a PhET-iO Standard Wrapper, it wouldn't fail out (is that correct)? It would just stick with the default that the Standard Wrapper was saved with. Correct, @zepumph?

That's correct, it will take the value from the state. If by "fail out" you mean assert in the wrapper frame (which won't error unless we have phetioWrapperDebug=true enabled).

[arouinfar]

on a full sim-scale locale, if the zh_CN translation doesn't exist at all, pop up the not a valid value,

I thought that we don't want this since we have started using ?locale to drive sim pages on the website. I can't exactly remember the conversation though. It worries me a bit to add that.

That was also my recollection. I thought the idea was that the valid values of locale are the full set of locale codes, not the available locales for a particular sim. So locale=zh_CN is always valid, but not all sims have been translated into Chinese (Simplified). It seems strange to trigger the Invalid Query Parameter dialog when the issue is a missing translation, as it implies that zh_CN isn't a real locale code. For reference, he's what the dialog typically looks like:

[kathy-phet]

JO and I talked through this ?locale behavior re when to and when not to pop up an error message in PhET Brand. Here is the suggested behavior:

If any supported format is entered "xx_XX", or "xx", or "xxx" (i would say we should be lenient and make these all case insensitive, @jonathanolson):

• never pop up a error message
• if available, run the sim in the exact language provided
• if not available, then
  -- if "xx_XX" was provided, see if "xx" is available and run that if available
  -- otherwise fall back to "en"
  -this allows another platform to always put ?locale=es_MX and then get es_MX when it is available or es when it is not

-NOTE: this is a small bit different from the "within-sim" fallback where one string is missing a locale.
-NOTE 2: this approach means we do not need to ship all of the possible locales, because we are just using a simple fall back heuristic for the query parameter country code aspect.

If something else is entered "espanol" or "swedish" or "12345":

• provide an error message that specifies supported formats
• e.g. "?locale query parameter accepts the following formats
  -- "xx" for ISO-639-1
  -- "xx_XX" for ISO-639-1 and a 2-letter country code
  -- "xxx" for ISO-639-2
• this is to give feedback to users (and partners) when they are using an incorrect format all together.

Does this sound like the best combo of actions to all? (@arouinfar noted in slack that "Sounds reasonable to me!"
Noted that on the phet-io wrapper, the locale will just be completely ignored without any errors - which I think is good.

[pixelzoom]

There are standards for locale fallback, most notably the ICU standard. PhET doesn't use the full ICU locale format (only language and country codes) but the fallback order you've described is consistent with that standard.

See also https://learn.microsoft.com/en-us/globalization/locale/fallback.

[oliver-phet]

It's good to have @mattpen 's website fallback issue tagged here. https://github.com/phetsims/website-meteor/issues/673

[jonathanolson]

Working copy snapshot in https://gist.github.com/jonathanolson/13813e2481f5e3bc860ab4e4f871c39a (changeset too large for a github comment), DO NOT COMMIT.

[samreid]

Things are looking good here. To help, I wanted to bring to your attention that build-a-nucleus went from green to red on this chipper commit:

phetsims/chipper@99db72a

build-a-nucleus : fuzz : unbuilt
http://128.138.93.172/continuous-testing/ct-snapshots/1715192070417/build-a-nucleus/build-a-nucleus_en.html?continuousTest=%7B%22test%22%3A%5B%22build-a-nucleus%22%2C%22fuzz%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1715192070417%22%2C%22timestamp%22%3A1715194883652%7D&brand=phet&ea&fuzz
Query: brand=phet&ea&fuzz
Uncaught Error: Assertion failed: emitting reentrant depth of 1000 seems like a infinite loop to me!
Error: Assertion failed: emitting reentrant depth of 1000 seems like a infinite loop to me!
at window.assertions.assertFunction (http://128.138.93.172/continuous-testing/ct-snapshots/1715192070417/assert/js/assert.js:28:13)
at assert (TinyEmitter.ts:191:20)
at emit (TinyStaticProperty.ts:56:9)
at notifyListeners (Node.ts:1556:30)
at onAccessAttempt (TinyStaticProperty.ts:31:9)
at get (TinyProperty.ts:61:16)
at value (Node.ts:2026:31)
at getBounds (Node.ts:3583:16)
at getHeight (Node.ts:3590:16)
at height (LayoutProxy.ts:470:98)

[samreid]

I'll add the migration rule for acid-base-solutions

[samreid]

OK I fixed acid-base-solutions. CT says we need to do the same for keplers-laws, projectile-data-lab, and projectile-sampling-distributions.

[zepumph]

Tagging code review issue: phetsims/chipper#1441

[zepumph]

MR is complete and published. Next steps on main in phetsims/chipper#1441.

[zepumph]

Noting that the final feature we landed on here is documented in phetsims/qa#1089 (comment).

[phet-dev]

Reopening because there is a TODO marked for this issue.

[zepumph]

I moved three TODOs over to phetsims/chipper#1441.