Rosetta should ignore the "a11y" key in strings files

[zepumph]

From #193,

We are currently on a branch adding support for a11y strings, via nested objects under the a11y key. Before we merge to master, we need to add code to rosetta that ignores the string key "a11y".
I don't know what this will entail, so I'm going to assign this over to @jbphet. Hopefully this is a small change that can be added to master and published to the server relatively soon so that I can finish up string conversions for all a11y strings files over in phetsims/friction#180 while things are fresh mind. Thanks!

[zepumph]

@samreid and I think that there is still more work to be done before doing this. Unassigning for now. @jbphet we will let you know when we are ready for this.

[zepumph]

@jbphet, work over in #193 has been wrapping up. We are ready here for rosetta to be set to ignore the a11y key at the top of any strings files. Ideally this would be somewhat factored out, in the off chance that we want to change that string key, or add another to ignore, etc. I'm not sure the difficulty, but most of the rest of this work (including merging it to master) is on hold until this issue is sorted out. I'm still not sure what type of priority that give this. I'll assign @ariel-phet in case he has opinions about it. Over to you.

[ariel-phet]

Seems "medium" to me - I do think this work should be done, @jbphet may want to confer with EM as to where it can fit in with his sonfication/a11y obligations.

[zepumph]

@jbphet, note that you can use the a11ystring-plugin branch for chipper and friction to build a version of friction from the en json that has nested a11y strings.

[jbphet]

I just discussed when to work on this with @kathy-phet and @emily-phet, and we decided that I could use the time that was allocated for Number Line Suite after the first dev version of Number Line: Integers goes into dev testing. The max time to spend on this is two calendar weeks, and if I can't get rosetta into shape where I can make these changes on master by then, I'll branch from the currently live SHA and add just the feature to ignore a11y strings. I'd really like to avoid doing this, since it will increase the complexity of managing rosetta.

[kathy-phet]

FYI - When I was demoing translation at a conference this week, I noticed that the a11y strings are appearing in Rosetta, should this be happening? It seemed like all of this discussion is around disabling that for now (although I cannot recall why we would want to disable it?)?

Raising priority on this for now, until I hear that Rosetta is working as expected and that it should be displaying the a11y strings in the picture above.

[kathy-phet]

@jbphet - Please review my comment above. Is this what is expected? That is, are we now ready for translators to translate a11y strings? If so, we should tell them! If not, we should hide these.

[jbphet]

@kathy-phet - I've looked into this, and those strings are actually from the "Keyboard Shortcuts" dialog. As such, it's probably appropriate that they are translatable and aren't identified as a11y strings. Please let me know if you disagree and, if so, we can create a new issue and track further discussion there.

[jbphet]

I spoke with @kathy-phet in person, and she's fine with the strings from the "Keyboard Shortcuts" dialog being available for translation. She just initially thought that some of the strings looked like those used for screen-reader-based descriptions and wanted to make sure that such strings weren't leaking into the translation interface before we're ready for it.

[zepumph]

@jbphet mentioned that he is close to working on this. I'll merge the chipper branch to master.

[jbphet]

Slack dialog with @zepumph about plan for this:

@jbphet🏢 10:07 AM I'm going to be working on a11y support in Rosetta, and I had an idea for testing that I wanted to run by you: What if we add some a11y strings to Chains once I've got the rosetta changes implemented so that we can do end-to-end testing? I'd probably need help from you or someone to add the strings in a legit way so that the build process doesn't complain about unused string. What do you think?

@zepumph 10:57 AM
I have to merge my changes in chipper master first. But that sounds good and reasonable.

@jbphet🏢 11:01 AM
Okay, cool. How about this for a plan then: I finish the changes to support a11y in Rosetta, test them locally as best I can, let you know that I'm ready, then you merge in the changes to Chipper and add some a11y strings to Chains and publish it, then I put the Rosetta changes on phet-server and we test that the strings don't show up to the user, no errors occur in Rosetta's log, and a new translation (of Chains) can be successfully submitted.

@zepumph 11:10 AM
Sounds good with one addendum on timing. All my changes to merge are just nested object support, mainly "a11y strings" are just a convention. All this is to say that I'm going to merge to master right now, and then the a11y string addition and chains publication can happen when you give the go ahead.
I think that will be faster and just as safe.

[zepumph]

Because of refactoring of UtteranceQueue into its own library, there were some changes needed in the friction a11ystring-plugin branch. I committed them above to get @jbphet into a testable place for friction on that branch.

[jbphet]

I used the a11ystring-plugin branch of friction that @zepumph describes above to test how Rosetta behaves when a11y strings are present prior to adding code to identify and exclude a11y strings. To make this test work, I added test-specific code to Rosetta to reference a built version of friction from this branch as well as the string file from the branch on GitHub. Long story short, it behaves exactly as we would like it to, meaning that it doesn't crash and it doesn't present the a11y strings to the user for translation. The reason is that, if the code finds a string key in the sim that doesn't appear in the English strings file, it doesn't show that string to the user, and right now Rosetta doesn't know how to look for nested strings. So, for instance, during the test Rosetta found the string key "a11y.moveBookWith" in the friction HTML file, but when it looked in the English string file from GitHub, it used .hasOwnProperty( 'ally.moveBookWith' ), which is false, since the string is stored in the file like this:

  "a11y": {
    "moveBookWith": {
      "value": "Move grabbed book or zoomed-in book up, left, down, or right with Arrow keys, or with letter keys W, A, S, or D."
    }

So, we could leave the code alone and meet the letter of the law for this GitHub issue. However, I think it would be much better to add code that explicitly ignores string keys that start with 'a11y.' and that adds an entry to the log when it does.

[jbphet]

The code to explicitly ignore a11y strings should now be in place in Rosetta and live on phet-server. There were quite a number of other changes made at the same time because Rosetta was in a broken state when this issue became a priority, so we'll need to keep an eye on it.

@zepumph - I believe that it is now safe to deploy sims with a11y keys in their string files, so I'm going to assign this to you. I would suggest leaving this open until such a deployment has been done and the behavior of Rosetta is verified, including at least one successful submission of a translation by an outside user on a sim where the a11y strings were present.

[zepumph]

We will go ahead with converting molarity to this new system, see phetsims/molarity#205 for more info. I'm ready to close this issue, and just create new ones if anything comes up, but over to @jbphet just to make sure that sounds good.

[jbphet]

@zepumph said:

I'm ready to close this issue, and just create new ones if anything comes up[.]

I'd prefer not to close this one just yet. I won't feel like we have fully closed the loop until we have tested the live version of Rosetta using a sim with a11y strings that has been deployed to the production web site. @zepumph - I'm happy to do this verification, but I'll need to know when that deployment has occurred. I'm assigning the issue back to you, please just send it back to me when the Molarity sim is live with a11y strings and I'll take it from there.

[jbphet]

I just received word that Molarity has been published with the a11y strings in place, so I tested that Rosetta is correctly ignoring the a11y by first going to the translation URL, selecting Molarity+German (resulting URL = https://phet.colorado.edu/translate/sim/molarity/de), and verifying that none of the a11y strings were presented to the user. I then examined Rosetta's output log on phet-server using the command sudo journalctl -t rosetta and verified that it included the expected entries about ignoring a11y strings. These were present and looked correct, here is an excerpt for future reference:

Feb 17 15:40:29 phet-server.int.colorado.edu rosetta[111057]: 2020-02-17 15:40:29 | info - intentionally skipping a11y string (will not be presented to user): a11y.helpContent.popUpListDescription
Feb 17 15:40:29 phet-server.int.colorado.edu rosetta[111057]: 2020-02-17 15:40:29 | info - intentionally skipping a11y string (will not be presented to user): a11y.helpContent.moveThroughDescription
Feb 17 15:40:29 phet-server.int.colorado.edu rosetta[111057]: 2020-02-17 15:40:29 | info - intentionally skipping a11y string (will not be presented to user): a11y.helpContent.changeChooseDescription
Feb 17 15:40:29 phet-server.int.colorado.edu rosetta[111057]: 2020-02-17 15:40:29 | info - intentionally skipping a11y string (will not be presented to user): a11y.helpContent.closeListDescription

All indications are that the ignoring of a11y strings by Rosetta is working correctly, so this issue can be closed.