Skip to content

Fix sr-Latn/sr_Latn handling (fixes #61) #62

Merged
merged 1 commit into from May 7, 2014

3 participants

@muffinresearch
Mozilla member

Note this change will impact likely impact existing projects using i18n-abide if they have a sr_LATN locale dir.

Issue was originally raised here: mozilla/zippy#111

@muffinresearch
Mozilla member

@ozten r?

@ozten ozten commented on the diff Feb 26, 2014
tests/i18n-tests.js
+ return i18n.languageFrom("en_US");
+ },
+ "the result should be en-US": function(topic) {
+ assert.equal(topic, "en-US");
+ }
+ },
+ "when processing sr-Cyrl-RS locale to a language": {
+ topic: function() {
+ return i18n.languageFrom("sr_Cyrl_RS");
+ },
+ "the result should be sr-RS": function(topic) {
+ assert.equal(topic, "sr-RS");
+ }
+ },
+});
+
@ozten
Mozilla member
ozten added a note Feb 26, 2014

Thanks for the tests!

Can we get a brief summary of what the core issue is and why the logic for handling a locale that splits into 3 parts is insufficient?

@muffinresearch
Mozilla member

Handling locales split into 3 isn't something I've modified (I just added tests to make sure that was still unaffected). The issue that's being addressed here is just that I've been informed that sr_LATN is incorrect it should be sr_Latn. Note whilst looking at this I noticed Django also had a similar problem: see https://code.djangoproject.com/ticket/12230

The bit that need careful handling is if any existing projects are using this so maybe this needs to be backwards compatible for lookup? What do you think?

@ozten
Mozilla member
ozten added a note Feb 26, 2014

Thanks for the context.

Aren't locales usually added as ll_UUUUU where l means lower case character and U means upper case character?
Here is an example for sr_LATN
http://svn.mozilla.org/projects/l10n-misc/trunk/sumo-test/sr_LATN/

@muffinresearch
Mozilla member

That seems to be an outlier. I'm not a l10n expert at all 😄 but looking at http://www.w3.org/International/questions/qa-choosing-language-tags#scriptsubtag the 'Latn' in sr-Latn is a script subtag. Whereas in something like en-US 'US' is a country and should be capitalized.

See also http://tools.ietf.org/html/bcp47#section-2.1.1 Which has this on :

[ISO15924] recommends that script codes use lowercase with the initial letter capitalized ('Cyrl' Cyrillic).

@muffinresearch
Mozilla member

As an aside before doing anything with this I'm going to do some testing with this branch on Spartacus and Zippy as they're using this for server-side and client-side translations respectively.

@zaach
zaach added a note Apr 16, 2014

Thanks @muffinresearch, we encountered this sr-Latn issue on FxA. Did you have a chance to test this out on your projects?

@muffinresearch
Mozilla member

This was the testing I did and the issues I saw in the projects I have #62 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ozten
Mozilla member
ozten commented Feb 26, 2014

If this makes things more consistent across our products, I'm all for it.

Update with your test research and if that checks out, I'm r+ and will merge.

@muffinresearch
Mozilla member

@ozten so I've had a play with this and the middleware complained about a missing locale dir as you'd expect because it couldn't resolve and find the sr_Latn dir. Once this was updated it worked fine in Zippy's case.

Fwiw: I've got a separate PR to update grunt-i18n-abide (mozilla/grunt-i18n-abide#18) to use localeFrom and languageFrom i18n-abide so that should keep those two in-sync.

@ozten
Mozilla member
ozten commented Mar 5, 2014

Thanks!

Should I merge?

Will it break existing projects? Should we add a notice or any docs?

@muffinresearch
Mozilla member

I'll re-run the test and note the steps to take as docs.

@muffinresearch
Mozilla member

Here's the steps I took for testing zippy after pointing it at this branch.

  • Use updated version of i18n-abide
  • Check middleware logging for error messages relating to JSON files not found e.g you'll see something like this: Bad locale=[sr_Latn] missing .po files in [/home/nanook/code/zippy/i18n/sr_Latn/messages.json]. See locale/README (Error: ENOENT, no such file or directory '/home/nanook/code/zippy/i18n/sr_Latn/messages.json')
  • Move affected directories e.g. (depending on where your directories are) mv locale/sr_{LATN,Latn} mv i18n/sr_{LATN,Latn}
  • Start-up site and check for errors.
  • Test affected locales.
  • Lastly ensure extraction and compilation tools are following the same directory format. (For grunt-i18n-abide an update is upcoming that uses the localeFrom/languageFrom direct from i18n-abide to keep them in-sync).
@muffinresearch
Mozilla member

@ozten any updates - do you need any more info?

@ozten ozten merged commit e469c31 into mozilla:master May 7, 2014

1 check passed

Details default The Travis CI build passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.