mozilla-services / FindMyDevice

Find My Device - 🚨🚨This server is obsolete and unsupported.🚨🚨
Mozilla Public License 2.0
13 stars 8 forks source link

Feature/langloc #304

Closed jrconlin closed 9 years ago

jrconlin commented 9 years ago

This will return a client.json file based on user preference. It will work from most preferred to least, to "en" as the default. It also presumes all pathnames are in lower case (because LCD)

@kitcambridge, @nchapman r?

ghost commented 9 years ago

Looks good, pending the two nits above. If @nchapman is happy, I'm happy.

nchapman commented 9 years ago

Looks good to me! Travis passed after a rerun so that's good too! I did notice that this doesn't have the server side localization hooked up yet. The Localize method should read from the appropriate server.json file and return the matching string. Should that happen in this PR or in another one?

jrconlin commented 9 years ago

Ah, so perhaps I'm unclear on the original request.

you want /1/l10n/client.json to pull the language based client.json file, and then have the Localize callback return a server.json from the same directory? Is that correct?

nchapman commented 9 years ago

@jrconlin yeah sorry I probably didn't make that clear enough. Yes, what you have going for client.json seems right on. That takes care of the strings that are used in the client side js app.

The strings in index.html (the marketing page) need to be replaced on the server side. That Localize function is currently just a stand in. Localize should take a string, look up that string in the parsed server.json file in the same directory and return the value. If the string isn't found it should just return the string that was passed to it (here's the js implementation for reference). There are 34 calls to the Localize function in index.html so parsing the appropriate server.json file once per request would be ideal.

jrconlin commented 9 years ago

@nchapman: Ok, added the server side config mods. Also added a few tests and a sample file to see if I'm doing it right. Feel free to point and laugh if I'm not.

@kitcambridge: r? on the deltas for 2e94186?

pdehaan commented 9 years ago

Is this ready to merge, or still a masterpiece in progress?

jrconlin commented 9 years ago

Just looking for the r+ from nchapman and we should be good. I think he has concerns about the stub "en" file.

nchapman commented 9 years ago

Yep. Ideally, it would be replaced by the real files, but without it the server won't even start up. That's kind of an issue for testing or for getting folks up and working quickly.

Since we always have the English strings embedded in our files, could we instead make it so that if it doesn't find the .../en/server.json file that it instead just loads up an empty hash? Bad idea?

nchapman commented 9 years ago

We decided to leave in the example server.json file and remove it later if it becomes an issue. This PR looks good to me! r+