Skip to content
This repository has been archived by the owner on Nov 22, 2022. It is now read-only.

Add french locale #77

Merged
merged 4 commits into from
Nov 28, 2016
Merged

Conversation

PerfectSlayer
Copy link
Collaborator

As the I18N support just landed, I add my native language 😉

Note: @bpierre The addon description is quite outdated… May be it's time to update it before more locales come?

@fluks
Copy link
Contributor

fluks commented Aug 23, 2016

Did you notice that you can also translate the preferences? See fi.properties for the syntax. Some of the strings of the default locale are in package.json, that's why fi and en-US properties files differ.

Should I add documentation what you can translate and how?

@bpierre bpierre mentioned this pull request Aug 23, 2016
@bpierre
Copy link
Owner

bpierre commented Aug 23, 2016

Note: @bpierre The addon description is quite outdated… May be it's time to update it before more locales come?

Do you have anything in mind? I created an issue to discuss it: #78

@PerfectSlayer
Copy link
Collaborator Author

@fluks Oups, I missed it! Thanks for pointing me.
No need for specific documentation as it's the default way of translating prefs, so already in the MDN documentation.
But the documentation shows the package.json file should not contains description, just pref name.
So the en-US.properties locale file should contains all the string to convert and no more issue 😉

@bpierre Nothing special. But it seems me the addon makes a lot more by now! 😅

@PerfectSlayer
Copy link
Collaborator Author

Ok, I have update French locale and complete the default US one.

@bpierre It should be okay now if you would like to merge.

@fluks
Copy link
Contributor

fluks commented Aug 24, 2016

Did you try to run with your patches, @PerfectSlayer? The preferences are ok with "general.useragent.locale": "fr-FR" in devprefs.json, but the addon itself doesn't work at all. The menu is not shown. Everything works if you rename fr-FR.properties to fr.properties and use fr locale.

I see Firefox only with fr locale at https://www.mozilla.org/en-US/firefox/all/. So, as I understand, general.useragent.locale should never be fr-FR? I used en and fi locales, when testing, with en-US the addon doesn't work and it's possible to have en-US.

The bug probably is that, if there are no translations for the browser's locale, the addon cannot fallback to use some other language.

So, if there are no translations for the browser's locale, drop the subcode (territory, country, region, whatever it is) part of the locale and if there's no translations for that either, use en.

But I need to look closer what the problem is. And open an issue.

@PerfectSlayer
Copy link
Collaborator Author

PerfectSlayer commented Aug 24, 2016

@fluks I tried my locale by renaming it en-US.propreties and everything works fine as far as I can remember…

I check my own addon locale folder and the French locale, which works fine, is named fr-FR.properties. It's correct according the SDK locale code documentation.

EDIT: Where did you read it's the standard way to test a locale?

@fluks
Copy link
Contributor

fluks commented Aug 27, 2016

EDIT: Where did you read it's the standard way to test a locale?

I don't remember, probably Stack Overflow or something. And I confirmed it that by changing general.useragent.locale, Firefox changes also translations. But I don't remember if the official documentation mentions it. Or do you mean this quote:

So, if there are no translations for the browser's locale, drop the subcode (territory, country, region, whatever it is) part of the locale and if there's no translations for that either, use en.

This is just what I think should be done.

@PerfectSlayer
Copy link
Collaborator Author

Ok, I did a test using master branch with general.useragent.locale = fr-FR and the add-on didn't load neither. So I don't think the issue comes from the French locale. 😞

Meanwhile, I add few improvements to some sentences. It should be fine to merge now.

@fluks
Copy link
Contributor

fluks commented Aug 29, 2016

It's not the French locale. It's any locale for which there are no translations in data/languages.json. I have a fix waiting to be pushed.

fluks pushed a commit to fluks/gtranslate that referenced this pull request Aug 29, 2016
The bug was that if Firefox's locale (browser.useragent.locale in
about:config) was such that there weren't translations for that locale
in data/languages.json file, gtranslate's menucontext didn't show up at
all.

To fix this, a support for a locale is tested and in case a locale isn't
supported, a fallback locale is used.

See PR bpierre#77 for additional conversation.
@bpierre
Copy link
Owner

bpierre commented Nov 28, 2016

Looks like fr is the one to use if we want to avoid what looks like a bug: https://discourse.mozilla-community.org/t/addon-localization-issue-for-fr-fr-with-sdk/3962

@bpierre bpierre merged commit 58b1f1c into bpierre:master Nov 28, 2016
bpierre added a commit that referenced this pull request Nov 28, 2016
See this PR for more discussion #77
@bpierre
Copy link
Owner

bpierre commented Nov 28, 2016

Merged, thanks!

@PerfectSlayer
Copy link
Collaborator Author

You're welcome! 👌

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants