Skip to content

Commit

Permalink
Bug 1346819 - Port SanitizeAsBCP47 to LocaleService. r=baku,jfkthame
Browse files Browse the repository at this point in the history
MozReview-Commit-ID: 2SXD5HaJPXr

--HG--
extra : rebase_source : 44404dfd577f13fde722ef5c13a29f853766786d
  • Loading branch information
Zibi Braniecki committed Mar 13, 2017
1 parent b28f0b3 commit fbfa499
Show file tree
Hide file tree
Showing 14 changed files with 120 additions and 33 deletions.
2 changes: 1 addition & 1 deletion addon-sdk/source/test/test-l10n-locale.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ exports.testPreferedOsLocale = function(assert) {
prefs.set(PREF_SELECTED_LOCALE, "");
prefs.set(PREF_ACCEPT_LANGUAGES, "");

let expectedLocale = Services.locale.getAppLocale().toLowerCase();
let expectedLocale = Services.locale.getAppLocaleAsLangTag().toLowerCase();
let expectedLocaleList = [expectedLocale];

// Add default "en-us" fallback if the main language is not already en-us
Expand Down
4 changes: 2 additions & 2 deletions dom/base/nsGlobalWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14929,10 +14929,10 @@ nsGlobalWindow::GetPaintWorklet(ErrorResult& aRv)
}

void
nsGlobalWindow::GetAppLocales(nsTArray<nsString>& aLocales)
nsGlobalWindow::GetAppLocalesAsBCP47(nsTArray<nsString>& aLocales)
{
nsTArray<nsCString> appLocales;
mozilla::intl::LocaleService::GetInstance()->GetAppLocales(appLocales);
mozilla::intl::LocaleService::GetInstance()->GetAppLocalesAsBCP47(appLocales);

for (uint32_t i = 0; i < appLocales.Length(); i++) {
aLocales.AppendElement(NS_ConvertUTF8toUTF16(appLocales[i]));
Expand Down
2 changes: 1 addition & 1 deletion dom/base/nsGlobalWindow.h
Original file line number Diff line number Diff line change
Expand Up @@ -951,7 +951,7 @@ class nsGlobalWindow : public mozilla::dom::EventTarget,
GetPaintWorklet(mozilla::ErrorResult& aRv);

void
GetAppLocales(nsTArray<nsString>& aLocales);
GetAppLocalesAsBCP47(nsTArray<nsString>& aLocales);

#ifdef ENABLE_INTL_API
mozilla::dom::IntlUtils*
Expand Down
2 changes: 1 addition & 1 deletion dom/tests/mochitest/chrome/test_window_getAppLocales.html
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<div id="content" style="display: none">
<script>

let appLocales = window.getAppLocales();
let appLocales = window.getAppLocalesAsBCP47();
ok(appLocales.length > 0, "getAppLocales returns at least one locale.");
is(appLocales[0], "en-US", "The first app locale should be en-US.");

Expand Down
2 changes: 1 addition & 1 deletion dom/webidl/Window.webidl
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,7 @@ partial interface Window {
* Example: ["en-US", "de", "pl", "sr-Cyrl", "zh-Hans-HK"]
*/
[Func="IsChromeOrXBL"]
sequence<DOMString> getAppLocales();
sequence<DOMString> getAppLocalesAsBCP47();

#ifdef ENABLE_INTL_API
/**
Expand Down
2 changes: 1 addition & 1 deletion intl/locale/DateTimeFormat.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ DateTimeFormat::Initialize()

mLocale = new nsCString();
nsAutoCString locale;
intl::LocaleService::GetInstance()->GetAppLocale(locale);
intl::LocaleService::GetInstance()->GetAppLocaleAsBCP47(locale);
mLocale->Assign(locale);

return NS_OK;
Expand Down
79 changes: 76 additions & 3 deletions intl/locale/LocaleService.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,43 @@ NS_IMPL_ISUPPORTS(LocaleService, mozILocaleService, nsIObserver)

mozilla::StaticRefPtr<LocaleService> LocaleService::sInstance;

/**
* This function transforms a canonical Mozilla Language Tag, into it's
* BCP47 compilant form.
*
* Example: "ja-JP-mac" -> "ja-JP-x-lvariant-mac"
*
* The BCP47 form should be used for all calls to ICU/Intl APIs.
* The canonical form is used for all internal operations.
*/
static void SanitizeForBCP47(nsACString& aLocale)
{
#ifdef ENABLE_INTL_API
// Currently, the only locale code we use that's not BCP47-conformant is
// "ja-JP-mac" on OS X, but let's try to be more general than just
// hard-coding that here.
const int32_t LANG_TAG_CAPACITY = 128;
char langTag[LANG_TAG_CAPACITY];
nsAutoCString locale(aLocale);
UErrorCode err = U_ZERO_ERROR;
// This is a fail-safe method that will set langTag to "und" if it cannot
// match any part of the input locale code.
int32_t len = uloc_toLanguageTag(locale.get(), langTag, LANG_TAG_CAPACITY,
false, &err);
if (U_SUCCESS(err) && len > 0) {
aLocale.Assign(langTag, len);
}
#else
// This is only really needed for Intl API purposes, AFAIK,
// so probably won't be used in a non-ENABLE_INTL_API build.
// But let's fix up the single anomalous code we actually ship,
// just in case:
if (aLocale.EqualsLiteral("ja-JP-mac")) {
aLocale.AssignLiteral("ja-JP");
}
#endif
}

/**
* This function performs the actual language negotiation for the API.
*
Expand Down Expand Up @@ -80,14 +117,27 @@ LocaleService::~LocaleService()
}

void
LocaleService::GetAppLocales(nsTArray<nsCString>& aRetVal)
LocaleService::GetAppLocalesAsLangTags(nsTArray<nsCString>& aRetVal)
{
if (mAppLocales.IsEmpty()) {
ReadAppLocales(mAppLocales);
}
aRetVal = mAppLocales;
}

void
LocaleService::GetAppLocalesAsBCP47(nsTArray<nsCString>& aRetVal)
{
if (mAppLocales.IsEmpty()) {
ReadAppLocales(mAppLocales);
}
for (uint32_t i = 0; i < mAppLocales.Length(); i++) {
nsAutoCString locale(mAppLocales[i]);
SanitizeForBCP47(locale);
aRetVal.AppendElement(locale);
}
}

bool
LocaleService::GetRequestedLocales(nsTArray<nsCString>& aRetVal)
{
Expand Down Expand Up @@ -335,7 +385,7 @@ CreateOutArray(const nsTArray<nsCString>& aArray)
}

NS_IMETHODIMP
LocaleService::GetAppLocales(uint32_t* aCount, char*** aOutArray)
LocaleService::GetAppLocalesAsLangTags(uint32_t* aCount, char*** aOutArray)
{
if (mAppLocales.IsEmpty()) {
ReadAppLocales(mAppLocales);
Expand All @@ -348,12 +398,35 @@ LocaleService::GetAppLocales(uint32_t* aCount, char*** aOutArray)
}

NS_IMETHODIMP
LocaleService::GetAppLocale(nsACString& aRetVal)
LocaleService::GetAppLocalesAsBCP47(uint32_t* aCount, char*** aOutArray)
{
AutoTArray<nsCString, 32> locales;
GetAppLocalesAsBCP47(locales);

*aCount = locales.Length();
*aOutArray = CreateOutArray(locales);

return NS_OK;
}

NS_IMETHODIMP
LocaleService::GetAppLocaleAsLangTag(nsACString& aRetVal)
{
if (mAppLocales.IsEmpty()) {
ReadAppLocales(mAppLocales);
}
aRetVal = mAppLocales[0];
return NS_OK;
}

NS_IMETHODIMP
LocaleService::GetAppLocaleAsBCP47(nsACString& aRetVal)
{
if (mAppLocales.IsEmpty()) {
ReadAppLocales(mAppLocales);
}
aRetVal = mAppLocales[0];
SanitizeForBCP47(aRetVal);
return NS_OK;
}

Expand Down
7 changes: 4 additions & 3 deletions intl/locale/LocaleService.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class LocaleService : public mozILocaleService,
* Use this accessor in C++ code that just wants to call a method on the
* instance, but does not need to hold a reference, as in
* nsAutoCString str;
* LocaleService::GetInstance()->GetAppLocale(str);
* LocaleService::GetInstance()->GetAppLocaleAsLangTag(str);
*/
static LocaleService* GetInstance();

Expand All @@ -88,11 +88,12 @@ class LocaleService : public mozILocaleService,
*
* Usage:
* nsTArray<nsCString> appLocales;
* LocaleService::GetInstance()->GetAppLocales(appLocales);
* LocaleService::GetInstance()->GetAppLocalesAsLangTags(appLocales);
*
* (See mozILocaleService.idl for a JS-callable version of this.)
*/
void GetAppLocales(nsTArray<nsCString>& aRetVal);
void GetAppLocalesAsLangTags(nsTArray<nsCString>& aRetVal);
void GetAppLocalesAsBCP47(nsTArray<nsCString>& aRetVal);

/**
* Returns a list of locales that the user requested the app to be
Expand Down
21 changes: 17 additions & 4 deletions intl/locale/mozILocaleService.idl
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,19 @@ interface mozILocaleService : nsISupports
*
* This API always returns at least one locale.
*
* When retrieving the locales for language negotiation and matching
* to language resources, use the language tag form.
* When retrieving the locales for Intl API or ICU locale settings,
* use the BCP47 form.
*
* Example: ["en-US", "de", "pl", "sr-Cyrl", "zh-Hans-HK"]
*
* (See LocaleService.h for a more C++-friendly version of this.)
*/
void getAppLocales([optional] out unsigned long aCount,
[retval, array, size_is(aCount)] out string aLocales);
void getAppLocalesAsLangTags([optional] out unsigned long aCount,
[retval, array, size_is(aCount)] out string aLocales);
void getAppLocalesAsBCP47([optional] out unsigned long aCount,
[retval, array, size_is(aCount)] out string aLocales);

/**
* Negotiates the best locales out of a ordered list of requested locales and
Expand Down Expand Up @@ -100,14 +107,20 @@ interface mozILocaleService : nsISupports
* The result is a valid locale ID and it should be
* used for all APIs that do not handle language negotiation.
*
* Where possible, getAppLocales() should be preferred over this API and
* When retrieving the locales for language negotiation and matching
* to language resources, use the language tag form.
* When retrieving the locales for Intl API or ICU locale settings,
* use the BCP47 form.
*
* Where possible, getAppLocales*() should be preferred over this API and
* all callsites should handle some form of "best effort" language
* negotiation to respect user preferences in case the use case does
* not have data for the first locale in the list.
*
* Example: "zh-Hans-HK"
*/
ACString getAppLocale();
ACString getAppLocaleAsLangTag();
ACString getAppLocaleAsBCP47();

/**
* Returns a list of locales that the user requested the app to be
Expand Down
2 changes: 1 addition & 1 deletion intl/locale/nsCollation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ NS_IMPL_ISUPPORTS(nsCollationFactory, nsICollationFactory)
nsresult nsCollationFactory::CreateCollation(nsICollation** instancePtr)
{
nsAutoCString appLocale;
mozilla::intl::LocaleService::GetInstance()->GetAppLocale(appLocale);
mozilla::intl::LocaleService::GetInstance()->GetAppLocaleAsLangTag(appLocale);

return CreateCollationForLocale(appLocale, instancePtr);
}
Expand Down
18 changes: 9 additions & 9 deletions intl/locale/tests/gtest/TestLocaleService.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,16 @@
using namespace mozilla::intl;


TEST(Intl_Locale_LocaleService, GetAppLocales) {
TEST(Intl_Locale_LocaleService, GetAppLocalesAsLangTags) {
nsTArray<nsCString> appLocales;
LocaleService::GetInstance()->GetAppLocales(appLocales);
LocaleService::GetInstance()->GetAppLocalesAsLangTags(appLocales);

ASSERT_FALSE(appLocales.IsEmpty());
}

TEST(Intl_Locale_LocaleService, GetAppLocales_firstMatchesChromeReg) {
TEST(Intl_Locale_LocaleService, GetAppLocalesAsLangTags_firstMatchesChromeReg) {
nsTArray<nsCString> appLocales;
LocaleService::GetInstance()->GetAppLocales(appLocales);
LocaleService::GetInstance()->GetAppLocalesAsLangTags(appLocales);

nsAutoCString uaLangTag;
nsCOMPtr<nsIToolkitChromeRegistry> cr =
Expand All @@ -32,9 +32,9 @@ TEST(Intl_Locale_LocaleService, GetAppLocales_firstMatchesChromeReg) {
ASSERT_TRUE(appLocales[0].Equals(uaLangTag));
}

TEST(Intl_Locale_LocaleService, GetAppLocales_lastIsEnUS) {
TEST(Intl_Locale_LocaleService, GetAppLocalesAsLangTags_lastIsEnUS) {
nsTArray<nsCString> appLocales;
LocaleService::GetInstance()->GetAppLocales(appLocales);
LocaleService::GetInstance()->GetAppLocalesAsLangTags(appLocales);

int32_t len = appLocales.Length();
ASSERT_TRUE(appLocales[len - 1].EqualsLiteral("en-US"));
Expand All @@ -48,12 +48,12 @@ TEST(Intl_Locale_LocaleService, GetRequestedLocales) {
ASSERT_TRUE(len > 0);
}

TEST(Intl_Locale_LocaleService, GetAppLocale) {
TEST(Intl_Locale_LocaleService, GetAppLocaleAsLangTag) {
nsTArray<nsCString> appLocales;
LocaleService::GetInstance()->GetAppLocales(appLocales);
LocaleService::GetInstance()->GetAppLocalesAsLangTags(appLocales);

nsAutoCString locale;
LocaleService::GetInstance()->GetAppLocale(locale);
LocaleService::GetInstance()->GetAppLocaleAsLangTag(locale);

ASSERT_TRUE(appLocales[0] == locale);
}
6 changes: 3 additions & 3 deletions intl/locale/tests/unit/test_localeService.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,15 @@ function run_test()
run_next_test();
}

add_test(function test_getAppLocales() {
add_test(function test_getAppLocalesAsLangTags() {
const localeService =
Components.classes["@mozilla.org/intl/localeservice;1"]
.getService(Components.interfaces.mozILocaleService);

const appLocale = localeService.getAppLocale();
const appLocale = localeService.getAppLocaleAsLangTag();
do_check_true(appLocale != "", "appLocale is non-empty");

const appLocales = localeService.getAppLocales();
const appLocales = localeService.getAppLocalesAsLangTags();
do_check_true(Array.isArray(appLocales), "appLocales returns an array");

do_check_true(appLocale == appLocales[0], "appLocale matches first entry in appLocales");
Expand Down
2 changes: 1 addition & 1 deletion toolkit/components/mozintl/mozIntl.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const localeSvc =
*/
function getLocales(locales) {
if (!locales) {
return localeSvc.getAppLocales();
return localeSvc.getAppLocalesAsBCP47();
}
return locales;
}
Expand Down
4 changes: 2 additions & 2 deletions toolkit/components/telemetry/TelemetryEnvironment.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -226,12 +226,12 @@ function enforceBoolean(aValue) {
}

/**
* Get the current browser.
* Get the current browser locale.
* @return a string with the locale or null on failure.
*/
function getBrowserLocale() {
try {
return Services.locale.getAppLocale();
return Services.locale.getAppLocaleAsLangTag();
} catch (e) {
return null;
}
Expand Down

0 comments on commit fbfa499

Please sign in to comment.