collision between query parameters and string keys

[pixelzoom]

While working on phetsims/molecule-polarity#5 and phetsims/molecule-polarity#7, I encountered some odd behavior. Molecule Polarity has 2 global options, presented to the user as 'dipole direction' and 'surface color'. Since all global options should be settable via query parameters, it makes sense that those query parameters should be named 'dipoleDirection' and 'surfaceColor'. It also makes sense that the string used to label these options should have keys 'dipoleDirection' and 'surfaceColor'. But apparently the string.js plugin reads query parameters to override string values. So when I set a query parameter, it's changing the strings that appear in my Options dialog.

I realize why this is being done. But this particular implementation presents the opportunity of unexpected collisions, especially since we can't possibly know all of the string keys that we might pull in from common code. Highly recommended to change this behavior, and override strings in another manner; either by qualifying the query parameter names when overriding strings (e.g., string.KEY=VALUE), or by attaching all overridden strings to 1 query parameter (e.g., strings="KEY=VALUE;...").

[pixelzoom]

@samreid @jbphet @jonathanolson Where is this feature (overriding strings via query parameters) currently being used?

[samreid]

The only use case for overriding strings with query parameter is https://github.com/phetsims/lingo/blob/master/lingo_en.html, you can test it at http://localhost:[your-port-here]/lingo/lingo_en.html

[pixelzoom]

Changes:

Overriding strings is now done via a single 'strings' query parameter, whose format is JSON that is identical to string.json files.

string.js reads the 'strings' query parameter, parses its JSON value, then uses these values to override strings.

lingo has been modified to use the 'strings' query parameter. (I also did some misc cleanup and generalization, so that lingo doesn't have a hardcoded URL and can be used with a working copy of energy-skate-park-basics.)

Assigning to @samreid to review.

[pixelzoom]

@jbphet please be aware of these changes when you're working on rosetta. If you're planning to specify translated strings via query parameters, you'll need to do so via this new method.

[jbphet]

Got it, thanks.

[pixelzoom]

@jonathanolson commented via Skype:
[1/16/15, 12:50:49 PM] Jonathan Olson: How do we do JSON-like strings entries via a query parameter without using commas?
[1/16/15, 2:03:25 PM] Jonathan Olson: Commas, slashes and colons are reserved in query components, so we've been trying to avoid using those. See http://stackoverflow.com/questions/2366260/whats-valid-and-whats-not-in-a-uri-query

I replied:
[1/16/15, 2:08:57 PM] Chris Malley: feel free to propose a different solution in #81. promoting string keys to query parameters (a) had the same problem and (b) was colliding with other legitimate query parameters.

[pixelzoom]

Concern about URI reserved characters has been addressed by using encodeURIComponent to create the 'strings' parameter value, and decodeURIComponent to the value in strings.js.

[pixelzoom]

More Skype discussion with @jonathanolson:

[1/16/15, 2:23:25 PM] Jonathan Olson: can write it in the issue, but what do people think about:
?string%21MOLECULE_SHAPES%2Foptions.showOuterLonePairs=50&.....
[1/16/15, 2:23:41 PM] Jonathan Olson: decodes to string!MOLECULE_SHAPES/options.showOuterLonePairs=50
[1/16/15, 2:24:14 PM] Jonathan Olson: or is it preferred to have string overrides done in one query parameter?
[1/16/15, 2:24:21 PM] Jonathan Olson: I don't have a strong preference
[1/16/15, 2:24:22 PM] Chris Malley: do you really expect to be manually typing these?
[1/16/15, 2:25:14 PM] Chris Malley: i prefer 1 query parameter.
[1/16/15, 2:25:15 PM] Jonathan Olson: not all the time, but if it's easy I'm sure I would use it
[1/16/15, 2:25:29 PM] Jonathan Olson: OK, will be fine with the one query parameter, properly encoded

[samreid]

I tried prefixing a query parameter in arch (changing it from ?log to ?phet.arch.log) and I quite liked that. Perhaps strings should be similarly prefixed? Something like phet.chipper.strings? Would make searching easier and reduce the risk of other stringy parameter collision in the future.

[samreid]

I tested latest lingo and checked the format for the query strings and everything looks good. Thanks for updating lingo @pixelzoom!

The only outstanding issue I'm aware of is whether we should change ?strings to ?phet.chipper.strings. Let's discuss it.

[jonathanolson]

I have a slight preference to just use ?strings (things like ?phet.scenery.ea&phet.scenery.sceneryLog=Instance&phet.scenery.sceneryStringLog instead of ?ea&sceneryLog=Instance&sceneryStringLog starts getting long if we apply it consistently.

If we pull in a library that checks query strings for something we're not namespacing, we may regret not changing earlier.

[samreid]

Good point @jonathanolson. Also, at the OER summit conference, we discovered that iframe embedding is particularly effective for putting sims into other content, even if it means doubly-nested iframes (though not tested on many platforms). So perhaps it is safe to assume PhET gets a frame all to itself and won't collide with other query parameters.

[pixelzoom]

How does a sim being in an iFrame prevent it from colliding with a query parameter that one of its dependencies uses?

[samreid]

How does a sim being in an iFrame prevent it from colliding with a query parameter that one of its dependencies uses?

It doesn't protect it from collisions with PhET Sim dependencies. But it does prevent collision with other 3rd party content that embed PhET simulations as an iframe dependency.

Since we get to choose our own dependencies this may be safe enough.

EDIT: sims=>3rd party content

[pixelzoom]

At 1/20/15 dev meeting, we decided to name the query parameter 'strings'.