phetsims / friction

"Friction" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/friction
GNU General Public License v3.0
4 stars 6 forks source link

appending a non-existent locale to standard wrapper does not load #341

Closed arouinfar closed 1 year ago

arouinfar commented 1 year ago

Related to https://github.com/phetsims/qa/issues/899

The problem described in https://github.com/phetsims/phet-io/issues/1901 exists in the latest Friction RC. image

This has already been corrected in master, but it's not obvious to me which commits need cherry-picking, so I recommend that @jessegreenberg consult with @samreid.

Additionally, I made a related change to phet-io-guide.md that will also need to be cherry-picked. https://github.com/phetsims/phet-io-sim-specific/commit/b3b518e231cfa737aa5e51b6533647c2f3f7ca11

jessegreenberg commented 1 year ago

Thanks @arouinfar. At first I thought this was https://github.com/phetsims/friction/issues/334, but that was about non-existent locales in studio. This is a different problem for the standard wrapper.

jessegreenberg commented 1 year ago

https://github.com/phetsims/phet-io-wrappers/commit/84da4632b21a7163b3e7922604d2ced29b5d4133 is the only commit referenced in https://github.com/phetsims/phet-io/issues/1901, @samreid is that all we need?

Then Ill also do https://github.com/phetsims/phet-io-sim-specific/commit/b3b518e231cfa737aa5e51b6533647c2f3f7ca11.

jessegreenberg commented 1 year ago

I think that https://github.com/phetsims/ph-scale/issues/274 lists what needs to be cherry-picked for this.

jessegreenberg commented 1 year ago

https://github.com/phetsims/ph-scale/issues/274#issuecomment-1402589411 and the following comments confirm that those two commits are all that are needed.

jessegreenberg commented 1 year ago

I had a merge conflict with https://github.com/phetsims/phet-io-sim-specific/commit/b3b518e231cfa737aa5e51b6533647c2f3f7ca11. I met with @arouinfar and she helped me resolve conflicts the best way for friction-1.6.

@arouinfar said to test passing ?locale=xy with and without ?phetioDebug=true for the standard wrapper after these changes are cherry-picked.

jessegreenberg commented 1 year ago

While testing locally I kept hitting an assertion running studio from compiled index about the namespace. @samreid identified that I needed https://github.com/phetsims/phet-core/commit/ee0545a7ebede234990e529d6322d9c934b1c098, Ill cherry-pick that into friction-1.6 SHAs as well.

jessegreenberg commented 1 year ago

OK, I verified in 1.6 SHAs that ?locale=xy in the standard wrapper does not cause any error or assertion. That is true even when running with ?phetioDebug=true, which matches the behavior described in https://github.com/phetsims/ph-scale/issues/274#issuecomment-1419998499.

Ready to verify. @arouinfar if you wouldn't mind verifying this one in rc.3, that would probably be best.

arouinfar commented 1 year ago

@jessegreenberg looks good in rc.3, but I'll leave it for QA to close during testing.

pixelzoom commented 1 year ago

... 3, but I'll leave it for QA to close during testing.

Then I'll changed the label to fixed-awaiting-deploy.

Nancy-Salpepi commented 1 year ago

This is fixed in rc.3. When adding a nonexistent locale, such as xy, to the standard wrapper's url, it is ignored and the sim opens in English. With ?phetioDebug=true, there is no assertion error and the sim opens in English. The client guide has also been updated:

If you specify a locale for which a translation does not exist, the locale query parameter is ignored and the simulation will retain the localeProperty used when creating the Standard PhET-iO Wrapper.

Closing.