Bug 1345957 - Tighten LocaleService::NegotiateLanguages input handling. r?jfkthame
MozReview-Commit-ID: 9ZxHI0RpYpi
--- a/intl/locale/LocaleService.cpp
+++ b/intl/locale/LocaleService.cpp
@@ -427,16 +427,17 @@ LocaleService::FilterMatches(const nsTAr
// no available locale will be found more than once.
AutoTArray<Locale, 100> availLocales;
for (auto& avail : aAvailable) {
availLocales.AppendElement(Locale(avail));
}
for (auto& requested : aRequested) {
if (requested.IsEmpty()) {
+ MOZ_ASSERT(!requested.IsEmpty(), "Locale string cannot be empty.");
continue;
}
// 1) Try to find a simple (case-insensitive) string match for the request.
auto matchesExactly = [&](Locale& aLoc) {
return requested.Equals(aLoc.AsString(),
nsCaseInsensitiveCStringComparator());
};
@@ -499,42 +500,51 @@ LocaleService::FilterMatches(const nsTAr
// 6) Try to match against a region as a range
requestedLocale.ClearRegion();
if (findRangeMatches(requestedLocale, true, true)) {
HANDLE_STRATEGY;
}
}
}
-bool
+void
LocaleService::NegotiateLanguages(const nsTArray<nsCString>& aRequested,
const nsTArray<nsCString>& aAvailable,
const nsACString& aDefaultLocale,
LangNegStrategy aStrategy,
nsTArray<nsCString>& aRetVal)
{
- // If the strategy is Lookup, we require the defaultLocale to be set.
+ MOZ_ASSERT(aDefaultLocale.IsEmpty() || Locale(aDefaultLocale).IsValid(),
+ "If specified, default locale must be a valid BCP47 language tag.");
+
if (aStrategy == LangNegStrategy::Lookup && aDefaultLocale.IsEmpty()) {
- return false;
+ NS_WARNING("Default locale should be specified when using lookup strategy.");
}
FilterMatches(aRequested, aAvailable, aStrategy, aRetVal);
if (aStrategy == LangNegStrategy::Lookup) {
+ // If the strategy is Lookup and Filtering returned no matches, use
+ // the default locale.
if (aRetVal.Length() == 0) {
- // If the strategy is Lookup and Filtering returned no matches, use
- // the default locale.
- aRetVal.AppendElement(aDefaultLocale);
+ // If the default locale is empty, we already issued a warning, so
+ // now we will just pick up the LocaleService's defaultLocale.
+ if (aDefaultLocale.IsEmpty()) {
+ nsAutoCString defaultLocale;
+ GetDefaultLocale(defaultLocale);
+ aRetVal.AppendElement(defaultLocale);
+ } else {
+ aRetVal.AppendElement(aDefaultLocale);
+ }
}
} else if (!aDefaultLocale.IsEmpty() && !aRetVal.Contains(aDefaultLocale)) {
// If it's not a Lookup strategy, add the default locale only if it's
// set and it's not in the results already.
aRetVal.AppendElement(aDefaultLocale);
}
- return true;
}
bool
LocaleService::IsAppLocaleRTL()
{
nsAutoCString locale;
GetAppLocaleAsBCP47(locale);
@@ -830,22 +840,18 @@ LocaleService::NegotiateLanguages(const
availableLocales.AppendElement(aAvailable[i]);
}
nsAutoCString defaultLocale(aDefaultLocale);
LangNegStrategy strategy = ToLangNegStrategy(aStrategy);
AutoTArray<nsCString, 100> supportedLocales;
- bool result = NegotiateLanguages(requestedLocales, availableLocales,
- defaultLocale, strategy, supportedLocales);
-
- if (!result) {
- return NS_ERROR_INVALID_ARG;
- }
+ NegotiateLanguages(requestedLocales, availableLocales,
+ defaultLocale, strategy, supportedLocales);
*aRetVal =
static_cast<char**>(moz_xmalloc(sizeof(char*) * supportedLocales.Length()));
*aCount = 0;
for (const auto& supported : supportedLocales) {
(*aRetVal)[(*aCount)++] = moz_xstrdup(supported.get());
}
--- a/intl/locale/LocaleService.h
+++ b/intl/locale/LocaleService.h
@@ -238,17 +238,17 @@ public:
*
* Strategy is one of the three strategies described at the top of this file.
*
* The result list is canonicalized and ordered according to the order
* of the requested locales.
*
* (See mozILocaleService.idl for a JS-callable version of this.)
*/
- bool NegotiateLanguages(const nsTArray<nsCString>& aRequested,
+ void NegotiateLanguages(const nsTArray<nsCString>& aRequested,
const nsTArray<nsCString>& aAvailable,
const nsACString& aDefaultLocale,
LangNegStrategy aLangNegStrategy,
nsTArray<nsCString>& aRetVal);
/**
* Returns whether the current app locale is RTL.
*/
--- a/intl/locale/MozLocale.cpp
+++ b/intl/locale/MozLocale.cpp
@@ -13,16 +13,18 @@
using namespace mozilla::intl;
/**
* Note: The file name is `MozLocale` to avoid compilation problems on case-insensitive
* Windows. The class name is `Locale`.
*/
Locale::Locale(const nsACString& aLocale)
{
+ MOZ_ASSERT(!aLocale.IsEmpty(), "Locale string cannot be empty");
+
int32_t position = 0;
if (!IsASCII(aLocale)) {
mIsValid = false;
return;
}
nsAutoCString normLocale(aLocale);
--- a/intl/locale/tests/gtest/TestLocaleServiceNegotiate.cpp
+++ b/intl/locale/tests/gtest/TestLocaleServiceNegotiate.cpp
@@ -25,8 +25,29 @@ TEST(Intl_Locale_LocaleService, Negotiat
LocaleService::GetInstance()->NegotiateLanguages(
requestedLocales, availableLocales, defaultLocale, strategy, supportedLocales);
ASSERT_TRUE(supportedLocales.Length() == 2);
ASSERT_TRUE(supportedLocales[0].EqualsLiteral("sr-Cyrl"));
ASSERT_TRUE(supportedLocales[1].EqualsLiteral("en-US"));
}
+
+TEST(Intl_Locale_LocaleService, UseLSDefaultLocale) {
+ nsTArray<nsCString> requestedLocales;
+ nsTArray<nsCString> availableLocales;
+ nsTArray<nsCString> supportedLocales;
+ nsAutoCString defaultLocale("");
+ LocaleService::LangNegStrategy strategy =
+ LocaleService::LangNegStrategy::Lookup;
+
+ requestedLocales.AppendElement(NS_LITERAL_CSTRING("sr"));
+
+ availableLocales.AppendElement(NS_LITERAL_CSTRING("de"));
+
+ LocaleService::GetInstance()->NegotiateLanguages(
+ requestedLocales, availableLocales, defaultLocale, strategy, supportedLocales);
+
+ nsAutoCString lsDefaultLocale;
+ LocaleService::GetInstance()->GetDefaultLocale(lsDefaultLocale);
+ ASSERT_TRUE(supportedLocales.Length() == 1);
+ ASSERT_TRUE(supportedLocales[0].Equals(lsDefaultLocale));
+}